Bug 14700 - Crash due to java objects being collected before C# objects
Summary: Crash due to java objects being collected before C# objects
Status: RESOLVED FIXED
Alias: None
Product: Android
Classification: Xamarin
Component: Mono runtime / AOT Compiler ()
Version: 4.8.x
Hardware: PC Windows
: --- normal
Target Milestone: ---
Assignee: Rodrigo Kumpera
URL:
Depends on:
Blocks:
 
Reported: 2013-09-12 15:58 UTC by Adam Kemp
Modified: 2013-11-19 12:58 UTC (History)
3 users (show)

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


Attachments
Example project (16.10 KB, application/x-zip-compressed)
2013-09-12 15:59 UTC, Adam Kemp
Details
Modified example with another way to crash the app (17.10 KB, application/x-zip-compressed)
2013-09-13 14:15 UTC, Adam Kemp
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 FIXED

Description Adam Kemp 2013-09-12 15:58:38 UTC
See the attached project for reference. To reproduce the crash just run the app and rotate the device. You will crash when you rotate.

We are experiencing crashes in which the Java object backing a C# object has been collected by the Dalvik VM, but the C# object has not been collected. In this specific case the C# class is a subclass of Drawable, and one of these objects has been made the BackgroundDrawable of a View. The View is held onto as a field of a Fragment, which is retained so that it can be reused.

When you rotate the device our Activity is destroyed and then recreated. The newly created Activity goes to its FragmentManager and finds the retained fragment that it had been using earlier. When the Fragment's OnCreateView method is called it reuses the view that it had created the first time (yes, I know this is unusual, but it is allowed, and it works). Then, when the view goes to draw it uses the custom Drawable, and it turns out that Drawable is no longer valid.

The thing that's going wrong here is that in between the destruction of the original Activity and the creation of the new one there is no C# root holding on to the fragment, and thus the View and its Drawable is unreachable as far as the C# GC can tell. In our Activity's OnDestroy we force a GC.Collect to force this issue to surface.

The problem is that while the Fragment isn't reachable from the C# side it is reachable indirectly via the FragmentManager, and shortly after the collection occurs it is resurrected. Unfortunately, at that point some of the objects hanging off of it have been destroyed on the Java side.

If you comment out the first line of TestActivity.cs then we change the behavior by forcing the Fragment to be held on to by a static field, which causes the C# GC to keep that object and all of its hair intact.

What we want to happen (somehow) is for the mono GC to be aware that a Fragment which has RetainInstance=true is held in memory by the FragmentManager, and it should be considered reachable until its OnDestroy method is called. Otherwise we have to kind of roll our own kind of fragment manager to keep track of all of the retained fragments in an explicit C# root so that they don't get collected. In our actual app this may be more difficult than in the attached test app.
Comment 1 Adam Kemp 2013-09-12 15:59:10 UTC
Created attachment 4860 [details]
Example project
Comment 2 Adam Kemp 2013-09-13 14:15:47 UTC
Created attachment 4868 [details]
Modified example with another way to crash the app

This problem is even worse than originally thought. Not only does a GC during a device rotation cause our Drawable to be collected, but also a GC at any other point (even while the Activity and Fragment are actively being used) can trigger the same incorrect behavior.

I have modified the example project to contain a custom view which just sets a custom drawable as its background. The custom view maintains an explicit reference to the drawable, but the only thing holding on to the custom view is its parent view, which is just a normal LinearLayout.

I also added a button, which triggers a GC and a redraw when it is clicked. Tapping on that button causes the app to crash in the same way as rotating the device. Even if we hold on to the fragment in a static, clicking the button crashes the app.

This is a showstopper issue for us. We cannot ship with this current behavior because at any time a GC could happen, and our app would crash the next time it tries to draw certain views. Our view hierarchies are complicated enough and dynamic enough that holding static references to every individual view would not be practical.
Comment 3 Jonathan Pryor 2013-09-13 15:14:54 UTC
Completely unrelated to the GC bug at hand...

This line is wrong: you shouldn't create UI items with the Application context:

                    _topView = new View(Application.Context);

See: http://www.doubleencore.com/2013/06/context
Comment 4 Adam Kemp 2013-09-13 15:55:58 UTC
It's not "wrong". It is unusual (as I mentioned in my initial comment), but it does work, and the possible side effects of not using the Activity as the context are acceptable in our use cases. We deliberately use the application's context in order to avoid the leaks that would be caused by reusing our views when the activity is destroyed and recreated. We reuse our views because they are expensive to recreate, and in certain states of our application we hold references to individual views (in order to push updates into them), and disconnecting and reconnecting those views while in that state would require a significant rearchitecture.

Do you have any reason to believe that this line is related to the crash we're seeing? We have not had problems with the rotation or the reuse of those views in any other circumstances. It's just this GC issue that we're hitting.
Comment 5 Jonathan Pryor 2013-09-13 16:49:43 UTC
> Do you have any reason to believe that this line is related to the crash

No, which is why I prefixed my comment with "Completely unrelated to the GC bug at hand..."
Comment 6 Adam Kemp 2013-09-13 16:53:55 UTC
Sorry, I missed that line. :)

Thanks for pointing it out anyway. It was deliberate in this case, but I know you were just trying to help.
Comment 7 Adam Kemp 2013-11-11 16:51:44 UTC
Has there been any activity on this? We are still hitting random crashes in our app at times because objects that are still accessible via C# have been collected by the Java GC. We have been able to work around some of these, but not all of them.

**We will not be able to ship our app without this issue being resolved.**
Comment 8 Rodrigo Kumpera 2013-11-12 17:47:28 UTC
Hi Adam,

Sorry for taking so long to answer your bug.

I just tried your test with version 4.10.1 and it did work flawlessly in Debug and Release modes in a Nexus 7 running Android 4.3.

Could you give it a try? There has been one fix in the GC code that looks very similar to your that was released in 4.8.4 and and 4.10.0.

If it still crashes for you, could you provide details on the hardware/software you're using for testing?

Regards,
Rodrigo
Comment 9 Adam Kemp 2013-11-13 11:46:24 UTC
I verified that the attached project does not crash with the latest tools. I have so far been unable to test our real project because the new tools seem to be unable to activate my account after upgrading. I'm going to have to work with support on that issue before I can test this in our real code.

Thanks for the update. I'll post here again once I'm able to test more.
Comment 10 Adam Kemp 2013-11-19 12:58:14 UTC
After resolving my licensing issues I was able to verify that this bug appears to be fixed in our app. However, we encountered another GC-related bug, which was reported here:

https://bugzilla.xamarin.com/show_bug.cgi?id=16343

I believe that other bug is separate from this, and so this bug can be changed to Resolved.