mastodon-sc / mastodon

Mastodon – a large-scale tracking and track-editing framework for large, multi-view images.
BSD 2-Clause "Simplified" License
66 stars 20 forks source link

Fix Mastodon Memory Leak #244

Closed maarzt closed 1 year ago

maarzt commented 1 year ago

Mastodon currently has a memory leak. The memory occupied by Mastodon is not freed when closing Mastodon.That means user using Mastodon inside Fiji will quickly run out of memory if he or she opens multiple Mastodon projects in a row.

The PR together with https://github.com/mastodon-sc/mastodon-graph/pull/14 fixes the memory leaks in Mastodon and therefor closes #225. The PR also adds a unit test to make sure that Mastodon can be properly garbage collected.

Notable side effects of this PR:

TODOs:

maarzt commented 1 year ago

@stefanhahmann Could you please do a functional test to see if the feature computation dialog still works as intended. I especially wonder what happens if the feature computation dialog is closed and reopened later on. Do any computed features get lost?

maarzt commented 1 year ago

@tpietzsch What are your thought's about https://github.com/mastodon-sc/mastodon/pull/244/commits/679d3a57cfec29b152cac0d7492b1af69a3791e3 is it a viable solution to replace the PainterThread like that in Mastodon.

(The core problem is that the PreferencesDialog does not call the stop() method of the contained TrackSchemePanel and DataDisplayPanel. The SettingsPage interface, which is also borrowed from some BDV libraries, provides no way to clean up resources if the PreferenceDialog is disposed.)

tinevez commented 1 year ago

Maybe it could be a replacement for the one in imglib2 as well no? The solution to memory leak seems to be general and has all the features of the imglib2 version.

tpietzsch commented 1 year ago

@tpietzsch What are your thought's about 679d3a5 is it a viable solution to replace the PainterThread like that in Mastodon.

Yes, @maarzt great solution, and I agree with @tinevez that we should use it everywhere

stefanhahmann commented 1 year ago

@stefanhahmann Could you please do a functional test to see if the feature computation dialog still works as intended. I especially wonder what happens if the feature computation dialog is closed and reopened later on. Do any computed features get lost?

@maarzt I performed the test you suggested, even with two Mastodon instances open in parallel. Everything seems to work as expected. No features seem to get lost.

maarzt commented 1 year ago

I think this PR is ready to be merged, with one exception. It depends on a SNAPSHOT version of mastodon-graph. So we would actually need to wait for the mastodon-graph release??

tinevez commented 1 year ago

We can cut a release now of the mastodon-graph. I feel like we will be ready for a whole new public beta again, soon.... What do you think?

stefanhahmann commented 1 year ago

We can cut a release now of the mastodon-graph. I feel like we will be ready for a whole new public beta again, soon.... What do you think?

Would be nice, if #213 could be merged for that release.

tinevez commented 1 year ago

Yes, we could merge all recent work. I did not find something satisfactory for the log window yet, so this will have to wait.

maarzt commented 1 year ago

I updated the parent pom-scijava and the bigdataviewer-core dependency, because that is required to satisfy the grabage collection tests. Ideally bigdataviewer-core 10.4.7 is on the update site for the next Mastodon release. But Mastodon works also with BDV-core version 10.4.3. The only difference for Mastodon is the memory leak issue being fixed in BDV-core 10.4.7.