[Android] Calling Android.Views.View.Post(Action) on a View that is detached and not then reattached causes the Action and RunnableImplementor to leak.

[sjordanGSS]

Description

Context: #15769

When calling Android.Views.View.Post(Action) the passed Action will be wrapped in a RunnableImplementor and passed to Android.Views.View.Post(IRunnable). The constructor for RunnableImplementor adds itself to a static dictionary (presumably to retain a reference to it within .NET), from which it is removed when IRunnable.Run() is called. If Run is never called, the RunnableImplementor is never removed from this dictionary and will persist for the lifetime of the app.
It is possible to enter this scenario when calling Android.Views.View.Post(Action) on a view that is not attached to a window (i.e. isn't part of the view hierarchy), as Android.Views.View.Post(IRunnable) will add the RunnableImplementor to a queue instead of passing it to the Handler: the contents of this queue are passed to the Handler when the View is attached to the view hierarchy, however if it is destroyed instead the queue is deleted and IRunnable.Run() is never called. When this happens, the RunnableImplementor, the Action and anything referenced by the Action will leak.

There is a method View.RemoveCallbacks(Action) to remove the RunnableImplementor associated with the passed Action from the dictionary, however this requires the end user to keep track of posted Actions themselves and prohibits the use of code like

view.Post(() => 
{
    // do something on the UI thread
});

as a reference to the Action must be retained. An Action passed in the above way would be impossible to clean up later.

This issue is especially problematic in views like CollectionView, where child views are constantly being created, detached and recycled as the view is scrolled. If these children use Android.Views.View.Post(Action) without considering their current attach status, there is a risk of the Action leaking.

Steps to Reproduce

I'm working on a proper repro which I will update this post with, but in the mean time here are some steps to reproduce this issue:

• Instantiate an Android.Views.View or derivative, but do not add it as a child to any other view
• Call View.Post(Action)
• Dispose the View, remove any references to it and run the garbage collector
• The RunnableImplementor, Action and anything referenced within the Action will leak.
• Calling View.Post(Action) repeatedly (e.g. in a loop, on a timer etc) will eventually cause the application to crash with the message global reference table overflow

Link to public reproduction project repository

No response

Version with bug

7.0.101

Is this a regression from previous behavior?

Not sure, did not test other versions

Last version that worked well

Unknown/Other

Affected platforms

Android

Affected platform versions

Tested on Android 7.1 and 11

Did you find any workaround?

Avoid using View.Post if there is the possibility that the View might not be reattached. In my code, I did this by instantiating another Handler in the derived view and Posting to this instead.

Relevant log output

No response

[jonathanpeppers]

Some details on what is going on here: dotnet/android#8900 (comment)

You might consider not using Post() if the View is not attached to a window (IsAttachedToWindow == false).

[jonpryor]

@sjordanGSS: I would also like to address the last bullet point in Steps to Reproduce:

• Calling View.Post(Action) repeatedly (e.g. in a loop, on a timer etc) will eventually cause the application to crash with the message global reference table overflow

This particular part is not necessarily solvable. Android has a GREF limit of ~52000 entries. If you try to create ~52000 GREFs in a way that doesn't allow the GC to collect things, then the app will crash, and there is nothing we can do about it:

var v = new View(context);
for (int i = 0; i < 53000; ++i) {
    int x = i;
    v.Post(() => Console.WriteLine(x));
}
// Boom; attempting to add 53k Actions to a View will require
// creating 53k `RunnableImplementor` instances, which will
// create 53k GREFs, which will cause a Very Bad Time™.

View.Post() isn't really required here; you can just as easily crash things with:

var list = new List<Java.Lang.Object>();
for (int i = 0; i < 53000; ++i) {
    var o = new Java.Lang.Object();;
    list.Add(o);
}

Admittedly, using View.Post() hides things a bit more, especially because View.Post() only "moves" the callbacks to the UI mainloop when the View is attached to a Window, and not before, so it's not at all obvious that View.Post() is actually caching the Action/Runnable "for later". That was "fun" to investigate ¹.

Footnotes

1. If you never add the View to a Window, then the Runnables
   just hang around until the GC decides to collect them:

   1. When a View is not attached to a Window, then
      View.mAttachInfo is null, so we call getRunQueue():

      public boolean post(Runnable action) {
          …
          getRunQueue().post(action);
          return true;
      }
      

   2. View.getRunQueue() creates a HandlerActionQueue, sets
      View.mRunQueue.

   3. The only place View.mRunQueue is "meaningfully used" is within
      View.dispatchAttachedToWindow(), which "transfers" the
      Runnables within the HandlerActionQueue to the UI handler.

   4. View.dispatchAttachedToWindow() is only executed when the
      View is attacked to a Window.

   ↩