Bug 25995 - SIGABRT using Filter
Summary: SIGABRT using Filter
Status: RESOLVED INVALID
Alias: None
Product: Android
Classification: Xamarin
Component: Mono runtime / AOT Compiler ()
Version: 4.20.0
Hardware: PC Windows
: Normal normal
Target Milestone: ---
Assignee: dean.ellis
URL:
Depends on:
Blocks:
 
Reported: 2015-01-13 21:48 UTC by Jeremy Kolb
Modified: 2015-10-26 10:06 UTC (History)
3 users (show)

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

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 Jeremy Kolb 2015-01-13 21:48:21 UTC
I have a filter powered by a v7 SearchView and if I type really quickly into the search view (both adding and deleting characters) eventually I get something like the following:

In mgmain JNI_OnLoad
01-13 21:42:47.240 E/dalvikvm( 4497): JNI ERROR (app bug): attempt to use stale global reference 0x1d400f56
01-13 21:42:47.240 E/dalvikvm( 4497): VM aborting
01-13 21:42:47.240 F/libc    ( 4497): Fatal signal 6 (SIGABRT) at 0x00001191 (code=-6), thread 5072 (Filter)
The program 'Mono' has exited with code 0 (0x0).


I have no idea how to debug this, any help would be appreciated.

I'm using mvvmcross and my adapter/filter (minus SearchFunc which is just a Contains() on the item) is:

    public abstract class SearchFilterAdapterBase : MvxAdapter, IFilterable
    {
        protected SearchFilterAdapterBase(Context context)
            : base(context)
        {}

        protected SearchFilterAdapterBase(Context context, IMvxAndroidBindingContext bindingContext)
            : base(context, bindingContext)
        {}

        protected abstract Filter CreateFilter();

        private Filter _filter;

        public Filter Filter
        {
            get { return this._filter ?? (this._filter = this.CreateFilter()); }
        }

        private readonly object _lock = new object();

        public object Lock
        {
            get { return this._lock; }
        }

        [MvxSetToNullAfterBinding]
        public IEnumerable FilteredSubset { get; set; }

        protected override void SetItemsSource(IEnumerable value)
        {
            lock (Lock)
            {
                this.FilteredSubset = null;
                base.SetItemsSource(value);
            }
        }

        public override int GetPosition(object item)
        {
            lock (Lock)
            {
                IEnumerable source = this.FilteredSubset ?? ItemsSource;
                return MvxEnumerableExtensions.GetPosition(source, item);
            }
        }

        public override object GetRawItem(int position)
        {
            lock (Lock)
            {
                IEnumerable source = this.FilteredSubset ?? ItemsSource;
                return MvxEnumerableExtensions.ElementAt(source, position);
            }
        }

        public override View GetView(int position, View convertView, ViewGroup parent)
        {
            lock (Lock)
            {
                return base.GetView(position, convertView, parent);
            }
        }

        public override int Count
        {
            get
            {
                lock (Lock)
                {
                    return MvxEnumerableExtensions.Count(this.FilteredSubset ?? ItemsSource);
                }
            }
        }
    }

    public class SearchFilter<T> : Filter
    {
        private readonly SearchFilterAdapterBase _adapter;

        public SearchFilter(SearchFilterAdapterBase adapter)
        {
            this._adapter = adapter;
        }

        public Func<T, string, bool> SearchFunc { get; set; }

        protected override FilterResults PerformFiltering(ICharSequence constraint)
        {
            var returnObject = new FilterResults();
            if (constraint == null)
                return returnObject;

            string c = constraint.ToString();
            if (c == null)
                return returnObject;

            var results = new List<T>();

            lock (this._adapter.Lock)
            {
                if (this._adapter.ItemsSource.Count() > 0)
                {
                    IEnumerable<T> source = this._adapter.ItemsSource as IEnumerable<T>;
                    if (source == null)
                        return returnObject;

                    results.AddRange(source.Where(o => SearchFunc(o, c)));

                    returnObject.Values = FromArray(results.Select(r => r.ToJavaObject()).ToArray());
                    returnObject.Count = results.Count;
                }
            }

            return returnObject;
        }

        protected override void PublishResults(ICharSequence constraint, FilterResults results)
        {
            lock (this._adapter.Lock)
            {
                if (this._adapter.ItemsSource.Count() == 0)
                {
                    // When there is nothing in the items source make sure
                    // the filter is null.
                    this._adapter.FilteredSubset = null;
                    this._adapter.NotifyDataSetInvalidated();
                }
                else
                {
                    var values = results.Values;
                    if (values == null)
                        return;

                    _adapter.FilteredSubset = values.ToArray<Java.Lang.Object>()
                        .Select(r => r.ToNetObject<T>()).ToList();
                }

                this._adapter.NotifyDataSetChanged();
            }
        }
    }
Comment 1 Jeremy Kolb 2015-01-14 00:09:08 UTC
I think I have this figured out.  There's a bug in Xamarin's implementation of Filter.InvokeFilter(string constraint).

Resharper tells me it's this:

    public void InvokeFilter(string constraint)
    {
      Java.Lang.String @string = constraint != null ? new Java.Lang.String(constraint) : (Java.Lang.String) null;
      this.InvokeFilter((ICharSequence) @string);
      if (@string == null)
        return;
      @string.Dispose();
    }

The @string.Dispose() call ends up causing the constraint's handle to go to IntPtr.Zero in the middle of PerformFiltering which is not cool (when a GC has occurred).  I think the solution is to drop the Dispose() which will let the ICharSequence variant deref the Java.Lang.String.

If I instead use filter.InvokeFilter(new Java.Lang.String(myDotNetString)) I don't run into any issues.
Comment 2 Jonathan Pryor 2015-01-14 16:50:25 UTC
> I think the solution is to drop the Dispose() which will let the
> ICharSequence variant deref the Java.Lang.String.

That's *a* solution; the "problem" with doing that is that it increases the GREF count, which has many knock-on effects: the emulator dies if you allocate more than 2000, GREFs contribute to GC collection overhead, etc.

We want to keep the GREF count as low as possible, and as part of that we try to delete GREFs if we don't think they've "leaked" -- which was previously assumed to be the case for the (string) overloads, with the assumption that Java code would be invoked, and Java wouldn't care if the GREF was deleted after the method invocation.

Looking at the binding, I'm not sure how any leakage is occurring: Android.Widget.Filter.InvokeFilter(ICharSequence) is non-virtual, and thus can't be overridden, so all GREFs should be sanely scoped here: InvokeFilter(string) allocates a GREF (via Java.Lang.String), invokes InvokeFilter(ICharSequence), which invokes the Java method, then InvokeFilter(string) deletes the allocated GREF.

The "problem" here is with the operation of Filter.filter():

https://github.com/android/platform_frameworks_base/blob/46d8444631b4b1253a76bfcc78a29d26014d022f/core/java/android/widget/Filter.java#L101-125

It allocates a new thread (`HandlerThread`) which invokes Filter.performFiltering() from that thread.

What this does is it introduces unexpected thread-sharing:

    T1: Allocates Java.Lang.String
        Calls Filter.filter()
        Calls RequestHandler.sendMessageDelays()
    T2: Calls Filter.performFiltering(CharSequence)
        Calls SearchFilterAdapterBase.PerformFiltering(ICharSequence)

At this point, because T1 hasn't had a chance to dispose of the string yet, the parmeter marshaling code for PerformFiltering() finds and reuses the GREF originally allocated on T1.

    T1: Filter.filter() returns.
        Calls String.Dispose()

At this point, things go bad.

    T2: Attempts to use the ICharSequence, which was just invalidated by T1
        Dies horribly.

That's the background.
Comment 3 Jonathan Pryor 2015-01-14 16:52:28 UTC
I'm leary of fixing this by removing the Dispose() call, as (again) that would increase GREFs for everything.

Furthermore, there is a workaround, as you discovered (even if it's not easily discoverable): manually allocate a Java.Lang.String and "eat" the GREF (until some future unspecified GC determines that it can be collected).

Long term, we need to rethink parameter marshaling. This requires some pondering.
Comment 4 Jeremy Kolb 2015-01-14 18:45:12 UTC
That sucks.  Is the only thing that makes this "special" the fact that it hands the value to another thread?
Comment 5 Jonathan Pryor 2015-01-15 15:27:08 UTC
> Is the only thing that makes this "special" the fact that it hands
> the value to another thread?

Not quite; the problem is it (1) hands the value over to another thread, which (2) performs a Java > Managed transition, which (3) looks up the handle value originally created by the other thread.

All three are required; if the value wasn't handed to another thread, it wouldn't matter. If it was handed to another thread but that other thread never invoked C# code, it wouldn't matter. If another thread invoked managed code but didn't pass the same object reference, it wouldn't matter.

Hidden data sharing is a hell of a thing. :-(

Again, I'll readily admit that this is a bug, but the ramifications of just removing that Dispose() call are something I *really* want to avoid.
Comment 6 Jeremy Kolb 2015-01-20 17:00:30 UTC
Unfortunately my workaround doesn't always work, it just makes it less likely to happen.  Insights is still giving me crash reports.
Comment 7 Jonathan Pryor 2015-01-20 21:48:32 UTC
> Unfortunately my workaround doesn't always work

Now *that* is curious behavior which I cannot readily explain (and doesn't immediately make sense). :-(

A testcase would be handy, though not necessary.

> Insights is still giving me crash reports.

Is it possible those crash reports are from un-updated copies? (I can hope and dream, right?)
Comment 8 Jeremy Kolb 2015-10-26 10:06:00 UTC
I'm going to close this.  I believe it was due to some threading issue on my part where an event handler was running after the object had been GCed.