Bug 3585 - NullReferenceException when converting types
Summary: NullReferenceException when converting types
Status: RESOLVED NOT_ON_ROADMAP
Alias: None
Product: Android
Classification: Xamarin
Component: Mono runtime / AOT Compiler ()
Version: 4.0
Hardware: PC Windows
: --- normal
Target Milestone: ---
Assignee: Jonathan Pryor
URL:
Depends on:
Blocks:
 
Reported: 2012-02-22 09:59 UTC by Andy Oliver
Modified: 2012-05-04 07:46 UTC (History)
2 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 NOT_ON_ROADMAP

Description Andy Oliver 2012-02-22 09:59:04 UTC
When converting collections of arrays of a custom PointF type to Android.Graphics.PointF a NullReferenceException is raised part-way through on emulator and target MID.

Target device is v2.3, minimum Android to target is v2.2.

Behaviour occurs when run via Visual Studio 2010 or MonoDevelop 2.8.6.5 with Mono for Android 4.0.3.85258578

Debug output including stack trace:

I/monodroid-gc(  688): environment supports jni NewWeakGlobalRef
I/mono-stdout(  688): creating 0
I/mono-stdout(  688): creating 1
I/mono-stdout(  688): creating 2
I/mono-stdout(  688): creating 3
I/mono-stdout(  688): creating 4
I/mono-stdout(  688): creating 5
I/mono-stdout(  688): creating 6
I/mono-stdout(  688): creating 7
I/mono-stdout(  688): creating 8
I/mono-stdout(  688): creating 9
I/mono-stdout(  688): converting 0
I/mono-stdout(  688): converting 1
I/mono-stdout(  688): converting 2
I/mono-stdout(  688): converting 3
I/mono-stdout(  688): converting 4
I/mono-stdout(  688): converting 5
I/mono    (  688): Stacktrace:
I/mono    (  688): 
I/mono    (  688):   at Android.Runtime.JNIEnv.NewGlobalRef (intptr) <0x00043>
I/mono    (  688):   at Java.Lang.Object.RegisterInstance (Android.Runtime.IJavaObject,intptr,Android.Runtime.JniHandleOwnership) <0x000ab>
I/mono    (  688):   at Java.Lang.Object.SetHandle (intptr,Android.Runtime.JniHandleOwnership) <0x00023>
I/mono    (  688):   at Android.Graphics.PointF..ctor (single,single) <0x0031b>
I/mono    (  688):   at NullReferenceResearch.Classes.Conversion.ToPointF (NullReferenceResearch.Classes.WPointF) <0x0005b>
I/mono    (  688):   at NullReferenceResearch.Classes.Conversion.<ToPointFs>b__0 (NullReferenceResearch.Classes.WPointF) <0x00013>
I/mono    (  688):   at System.Linq.Enumerable/<CreateSelectIterator>c__Iterator12`2.MoveNext () <0x0015b>
I/mono    (  688):   at System.Collections.Generic.List`1.AddEnumerable (System.Collections.Generic.IEnumerable`1<T>) <0x000ab>
I/mono    (  688):   at System.Collections.Generic.List`1..ctor (System.Collections.Generic.IEnumerable`1<T>) <0x00093>
I/mono    (  688):   at System.Linq.Enumerable.ToArray<TSource> (System.Collections.Generic.IEnumerable`1<TSource>) <0x000c3>
I/mono    (  688):   at NullReferenceResearch.Classes.Conversion.ToPointFs (NullReferenceResearch.Classes.WPointF[]) <0x000ab>
I/mono    (  688):   at NullReferenceResearch.Activity1.OnCreate (Android.OS.Bundle) <0x00243>
I/mono    (  688):   at Android.App.Activity.n_OnCreate_Landroid_os_Bundle_ (intptr,intptr,intptr) <0x00057>
I/mono    (  688):   at (wrapper dynamic-method) object.dc4b54b9-ccb6-4bfb-bb52-cfd46d1695df (intptr,intptr,intptr) <0x00033>
I/mono    (  688):   at (wrapper native-to-managed) object.dc4b54b9-ccb6-4bfb-bb52-cfd46d1695df (intptr,intptr,intptr) <0xffffffff>


I was able to reproduce this behaviour with the code below:

---- Activity1.cs ----
using Android.App;
using Android.Graphics;
using Android.OS;
using NullReferenceResearch.Classes;
using Debug = System.Diagnostics.Debug;

namespace NullReferenceResearch
{
    [Activity(Label = "NullReferenceResearch", MainLauncher = true, Icon = "@drawable/icon")]
    public class Activity1 : Activity
    {
        private const int COLLECTION_SIZE = 10;
        private const int POINT_SIZE = 360;

        protected override void OnCreate(Bundle bundle)
        {
            base.OnCreate(bundle);

            // Set our view from the "main" layout resource
            SetContentView(Resource.Layout.Main);

            WPointCollection[] collection = new WPointCollection[COLLECTION_SIZE];

            for (int i = 0; i < collection.Length; i++)
            {
                Debug.WriteLine(@"creating {0}", i);
                WPointF[] newPoints = new WPointF[POINT_SIZE];
                for (int j = 0; j < newPoints.Length; j++)
                    newPoints[j] = new WPointF((float)j, (float)j * i);
                collection[i] = new WPointCollection(i, newPoints);
            }

            for (int i = 0; i < collection.Length; i++)
            {
                Debug.WriteLine(@"converting {0}", i);
                PointF[] pointFs = collection[i].Points.ToPointFs();
            }
        }
    }
}
 ----/Activity1.cs ----

---- WPointF.cs ----
using System.Linq;
using Android.Graphics;

namespace NullReferenceResearch.Classes
{
    public static class Conversion
    {
        public static PointF ToPointF (this WPointF wpointf)
		{
			PointF rv = new PointF (wpointf.X, wpointf.Y);
			return rv;
		}

        public static WPointF ToWPointF(this PointF pointf)
        {
            return new WPointF(pointf.X,pointf.Y);
        }

        public static PointF[] ToPointFs(this WPointF[] wpointfs)
        {
            return wpointfs.Select(w => w.ToPointF()).ToArray();
        }

        public static WPointF[] ToWPointFs(this PointF[] pointfs)
        {
            return pointfs.Select(p => p.ToWPointF()).ToArray();
        }
    }

    public class WPointCollection
    {
        public int Id { get; set; }
        public WPointF[] Points;

        public WPointCollection(int id, WPointF[] points)
        {
            Id = id;
            Points = points;
        }
    }

    public class WPointF
    {
        public float X;
        public float Y;

        public WPointF(float x, float y)
        {
            X = x;
            Y = y;
        }
    }
}
---- /WPointF.cs ----
Comment 1 Jonathan Pryor 2012-04-26 10:54:36 UTC
Presumably you're getting the NRE on the emulator, as you're running out of global references.

    http://docs.xamarin.com/android/troubleshooting#Unexpected_NullReferenceExceptions

I'm able to run this just fine on my Xoom, but it consumes 3603 global references to do so:

    http://docs.xamarin.com/android/advanced_topics/diagnostics
> I/monodroid-gref(32283): +g+ grefc 3605 gwrefc 0 obj-handle 0x4071d1a0/L -> new-handle 0x4071d1a0/L from    at Java.Lang.Object.RegisterInstance(IJavaObject instance, IntPtr value, JniHandleOwnership transfer)

Why does it require 3600 global references? Because your app logic requires it. Your `collection` initialization loop creates 3600 WPointF instances:

    for (int i = 0; i < collection.Length; i++)
    {
        Debug.WriteLine(@"creating {0}", i);
        WPointF[] newPoints = new WPointF[POINT_SIZE];
        for (int j = 0; j < newPoints.Length; j++)
            newPoints[j] = new WPointF((float)j, (float)j * i);
        collection[i] = new WPointCollection(i, newPoints);
    }
    // 3600 WPointF instances now exist in the GC heap

This is fine, as each WPointF doesn't require a gref. It's when you convert these into Android.Graphics.PointF instances that things go south:

    for (int i = 0; i < collection.Length; i++)
    {
        Debug.WriteLine(@"converting {0}", i);
        PointF[] pointFs = collection[i].Points.ToPointFs();
    }
    // 3600 PointF instances now exist

The question is, do you actually need 3600 PointF instances? In this code you clearly don't, as you're not using them. There are thus two workarounds:

1. Collect them!

    for (int i = 0; i < collection.Length; i++)
    {
        Debug.WriteLine(@"converting {0}", i);
        PointF[] pointFs = collection[i].Points.ToPointFs();
        GC.Collect();
    }

This uses at most ~720 grefs (though I could be wrong, I'm just skimming the output), and the collector brings that down to ~360 by the end of the loop.

This of course isn't ideal, because you normally don't want GC collections in the middle of loops, but for large & complicated object graphs it's frequently the simplest answer.

Note: Mono for Android 4.1.0 will automatically call GC.Collect() (with a warning message) when you hit 1800 grefs, so your app as-is should Just Work on 4.1.0 and later. It's still not ideal -- there are automagic GC collections involved -- but it'll work.

2. If you know you won't be using them anymore, Dispose() of them!

    for (int i = 0; i < collection.Length; i++)
    {
        Debug.WriteLine(@"converting {0}", i);
        PointF[] pointFs = collection[i].Points.ToPointFs();
        foreach (var p in pointFs)
            p.Dispose();
    }

This has better performance, as the GC doesn't need to run as frequently. By the end of the loop only 6 grefs are outstanding, though it does require 366 maximum grefs "in-flight".
Comment 2 Andy Oliver 2012-05-04 03:15:57 UTC
Thanks for your response.

The above is based on inherited code that is being ported from Win CE to both iOS Android. 

Needless to say the original code works fine on Win32, WinCE and MonoTouch/iOS, but fails on Mono for Android! Ten groups of 360 points on a circle is at the low end of reality. In some cases it could be as high as 100 -- and still works on WinCE, albeit slowly!

It's good to know that this will run in 4.1.0.

Thanks again for your input.
Comment 3 Jonathan Pryor 2012-05-04 07:46:47 UTC
I'm fairly sure that your original code wasn't using Android.Graphics.Point. :-)

The only type that could work w/o change on Win32, WinCE, and MonoTouch is System.Drawing.PointF, which will not have this issue. Mono for Android also provides System.Drawing.PointF, within OpenTK.dll. The forthcoming 4.2.0 release will be moving the System.Drawing types into Mono.Android.dll.

Furthermore, what are you doing with these Android.Graphics.PointF instances? Android.Graphics.Canvas doesn't use PointF (and Canvas isn't "fast"). OpenTK's drawing methods doesn't use PointF either.