Bug 20532 - Activities not disposed after OnDestroy. Seems to prevent 'normal' GC from happening
Summary: Activities not disposed after OnDestroy. Seems to prevent 'normal' GC from ha...
Status: RESOLVED INVALID
Alias: None
Product: Android
Classification: Xamarin
Component: Mono runtime / AOT Compiler ()
Version: 4.12.4
Hardware: PC Windows
: Normal normal
Target Milestone: ---
Assignee: Jonathan Pryor
URL:
Depends on:
Blocks:
 
Reported: 2014-06-11 11:05 UTC by Nathan Randle
Modified: 2014-06-11 12:37 UTC (History)
2 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
Simple sample app (18.16 KB, application/x-zip-compressed)
2014-06-11 11:05 UTC, Nathan Randle
Details
Heap dump when using the app without Dispose() in OnDestroy override (7.07 MB, application/octet-stream)
2014-06-11 11:06 UTC, Nathan Randle
Details
Heap dump when using the app with Dispose() (6.78 MB, application/octet-stream)
2014-06-11 11:07 UTC, Nathan Randle
Details
Memory analyzer showing the difference between Dispose and no-dispose (51.99 KB, image/png)
2014-06-11 11:11 UTC, Nathan Randle
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED INVALID

Description Nathan Randle 2014-06-11 11:05:08 UTC
Created attachment 7041 [details]
Simple sample app

After an activity has been destroyed with Xamarin Android it appears it will not be GC'd automatically unless Dispose() is called on the activity. Should Xamarin be calling Dispose() after OnDestroy?

I have attached a simple sample app to demonstrate this behaviour and captured heap dumps which illustrate the problem. When the overriden OnDestroy method in SecondActivity is uncommented the activity gets GC'd almost immediately after backing out of the view. If the Dispose call is commented then several instances of the activity remain on the heap. Strangely they DO get garbage collected if GC is manually invoked in .NET code. They don't seem to get GC'd if not manually invoked even after significant amount of time and lots of navigation within the app occurs (With the attached sample app we have heap dumps with over 50 instances on the heap)

See the attached heap dumps and the screenshots of Eclipse Memory Analyzer to see the difference when the Dispose() call is included and when not.

We also created a similar java dalvik app and the activities are collected instantly in the same way as the uncommented Dispose() case.
Comment 1 Nathan Randle 2014-06-11 11:06:32 UTC
Created attachment 7042 [details]
Heap dump when using the app without Dispose() in OnDestroy override
Comment 2 Nathan Randle 2014-06-11 11:07:53 UTC
Created attachment 7043 [details]
Heap dump when using the app with Dispose()
Comment 3 Nathan Randle 2014-06-11 11:11:10 UTC
Created attachment 7044 [details]
Memory analyzer showing the difference between Dispose and no-dispose
Comment 4 Jonathan Pryor 2014-06-11 12:37:27 UTC
This behavior is By Design.

http://developer.xamarin.com/guides/android/advanced_topics/garbage_collection/

The GC is a "normal" on-demand mark-and-sweep collector (no reference counting). It will not execute unless explicitly requested via GC.Collect(), or because it "needs" to because it's "out of memory." (Oversimplifying greatly.)

A Java.Lang.Object subclass instance maintains a persistent reference to the corresponding Java instance, ensuring that the Java instance will live at least as long as the Java.Lang.Object instance. The GC in place works such that when the GC runs, if there are no references to a Java.Lang.Object instance, then the corresponding Java instance will be released, allowing the Java GC to collect it.

The behavior you observe is the natural consequence of the GC being "lazy" (only invoked explicitly or "when `new` requires it):

1. The Xamarin.Android GC only "sees" "small" instances which don't consume much C# memory.
2. Few objects are being allocated from Mono's GC heap (few/no `new` statements or implicit object creation).
3. As a result of (1) and (2), the GC won't execute (as there's no need).
4. Because the GC doesn't execute, the Java instances are never released.
5. Because the Java instances aren't released, your memory profiling tools see them remaining on the heap, "unreferenced." (Except they _are_ referenced, by the managed wrapped instances.)

The workaround, as you note, is to do one of two things:

1. Explicitly invoke GC.Collect(). This causes the GC to run, which will collect the Java.Lang.Object wrapper instances, thus allowing the Java instances to be released.

2. Explicitly call Java.Lang.Object.Dispose(), which immediately releases the Java instance.

http://developer.xamarin.com/guides/android/advanced_topics/garbage_collection/#Helping_the_GC

> Should Xamarin be calling Dispose() after OnDestroy?

No.

First, how would this be done? The "obvious" thing to do would be to have the Activity.OnDestroy() binding call Dispose().

The problem with this is that overriding implementations need to call the base method:

    protected override void OnDestroy()
    {
        base.OnDestroy();
        // Do something with the Activity instance.
    }

If the base Activity.OnDestroy() method calls Dispose(), doing ANYTHING that involves Object.Handle after `base.OnDestroy()` will result in an exception, because Object.Handle will not be valid. To make it work you would need make `base.OnDestroy()` the LAST thing the method does, which ~nobody does or will think to do. (It's most common to do `base` method invocations at the start of the method, not the end.)

Alternatively, instead of having Activity.OnDestroy() invoke Dispose(), we could have the "marshal method" Activity.n_OnDestroy() invoke Dispose(). This way it wouldn't matter when base.OnDestroy() was invoked.

The problem with having the marshal method call Dispose() is that it breaks expectations. Imagine that you have Java code which calls Activity.onDestroy() _multiple times on the same instance_. (Doesn't matter _why_; it could be for test purposes, or because of a bug. The primary thing to realize is that nothing _prevents_ Java code from calling Activity.onDestroy() multiple times on the same instance.)

If Activity.n_OnDestroy() implicitly called Dispose(), this scenario would BREAK, as the _first_ invocation would work, and would then destroy the link between the C# Activity instance and the Java "peer" instance, and the _second_ invocation would FAIL, because there'd be no C# instance associated with the Java instance.

By keeping the Dispose() call in developer-code, the developer (you) can reason about what's going on and react accordingly, instead of having "magic logic" in the core runtime introducing corner cases and complicating debugging.