This is Xamarin's bug tracking system. For product support, please use the support links listed in your Xamarin Account.
Bug 12212 - iPhoneOSGameView.Run throws "An element with the same key already exists in the dictionary."
: iPhoneOSGameView.Run throws "An element with the same key already exists in t...
Status: VERIFIED FIXED
Product: iOS
Classification: Xamarin
Component: Runtime
: 6.2.x
: PC Windows
: High major
: Untriaged
Assigned To: Sebastien Pouliot
:
:
:
:
  Show dependency treegraph
 
Reported: 2013-05-12 18:55 EDT by randyficker
Modified: 2013-06-14 04:41 EDT (History)
4 users (show)

See Also:
Tags:
Test Case URL:
External Submit: ---


Attachments
Repro (5.15 KB, text/plain)
2013-05-17 03:43 EDT, randyficker
Details

Description randyficker 2013-05-12 18:55:58 EDT
Rarely, I get this error when calling Run:

System.ArgumentException: An element with the same key already exists in the
dictionary.
      at
System.Collections.Generic.Dictionary`2[OpenTK.ContextHandle,System.WeakReference].Add
(ContextHandle key, System.WeakReference value) [0x00000] in <filename
unknown>:0 
      at OpenTK.Graphics.GraphicsContext..ctor (OpenTK.Graphics.GraphicsMode
mode, IWindowInfo window, Int32 major, Int32 minor, GraphicsContextFlags flags)
[0x00000] in <filename unknown>:0 
      at OpenTK.Platform.Utilities.CreateGraphicsContext
(OpenTK.Graphics.GraphicsMode mode, IWindowInfo window, Int32 major, Int32
minor, GraphicsContextFlags flags) [0x00000] in <filename unknown>:0 
      at OpenTK.Platform.Utilities.CreateGraphicsContext (EAGLRenderingAPI
version) [0x00000] in <filename unknown>:0 
      at OpenTK.Platform.iPhoneOS.iPhoneOSGameView.CreateFrameBuffer ()
[0x00000] in <filename unknown>:0 
      at OpenTK.Platform.iPhoneOS.iPhoneOSGameView.RunWithFrameInterval (Int32
frameInterval) [0x00000] in <filename unknown>:0 
      at OpenTK.Platform.iPhoneOS.iPhoneOSGameView.Run () [0x00000] in
<filename unknown>:0 
      at WAML.IOS.RenderBoxImplementation.Start () [0x00000] in <filename
unknown>:0 

I haven't been able to repro it, but it's happened quite a few times.  It seems
to be random.  Saw in mtouch 6.2.3.0 (8d98f5e).

Googling it showed someone else that ran into this problem too, but his post to
the forums did not receive any replies:
http://monotouch.2284126.n4.nabble.com/Modification-of-the-OpenGLESSample-GameView-td4615982.html
Comment 1 Miguel de Icaza 2013-05-13 18:06:16 EDT
Hello Randy,

Would you happen to have a test case we can use to reproduce this?
Comment 2 Miguel de Icaza 2013-05-13 18:08:10 EDT
Reading the source code, and based on your comment that this happens randomly,
I wonder if you are using threads in your application, and just two threads try
to create the GraphcisConext at once?
Comment 3 randyficker 2013-05-17 03:43:11 EDT
Okay, I spent some more time on this, and I can now provide a repro. See
attachment.

And no, it doesn't appear to be related to multi-threading.  My app only ever
calls Run from the UI thread, as does the attached sample.

The attached sample is just a simple app that creates and destroys
iPhoneOSGameViews rapidly.  If you let it run long enough on an actual device,
the above error happens (usually within 15-30 seconds for me.)
Comment 4 randyficker 2013-05-17 03:43:28 EDT
Created attachment 3982 [details]
Repro
Comment 5 Miguel de Icaza 2013-05-19 11:00:11 EDT
Randy,

Notice that NSTimer runs on a parallel thread to the UI thread, so this means
that you are calling the new UIViewController from a background thread.

Can you rename your Tick () function with Tick2 (), and then make Tick () be:

Tick ()
{
    BeginInvokeOnMainThread (() => Tick2 ());
}
Comment 6 randyficker 2013-05-19 21:02:55 EDT
I don't believe that is true.  If I write out the
Thread.CurrentThread.ManagedThreadID from a NSTimer callback, it always shows
it's on the UI thread.

Nevertheless, I tried using BeginInvokeOnMainThread anyway as you suggested and
I still got the same error.
Comment 7 Miguel de Icaza 2013-05-26 16:13:08 EDT
Sebastien, would you mind researching this issue?
Comment 8 Sebastien Pouliot 2013-05-27 10:37:57 EDT
I can duplicate it and it does not seems to be threading related.

The sample creates a lot of new GraphicContext and the same context pointer can
be (re)used. 

Right now the code does not check if the WeakReference is alive before adding
it to the dictionary. If it's not alive it should replace the existing entry.
That's the "direct" cause of the exception.

Now if the WeakReference is not alive then it must have been disposed - and
that should have removed the entry from available_contexts. OTOH my breakpoint
on Dispose was not hit - and that's likely the "real" bug.
Comment 9 Sebastien Pouliot 2013-05-27 13:33:07 EDT
The main dispose issue is the lack of a finalizer, making instance freed by the
GC (and not manually disposed), not removed from the dictionary.

So adding

        ~GraphicsContext ()
        {
            this.Dispose(false);
        }

removes entries from `available_contexts`. That makes the situation better but
it's still possible that the pointer is part of the dictionary (but it's weak
reference will be alive). Looking into that...
Comment 10 Sebastien Pouliot 2013-05-27 14:32:20 EDT
The old, still in the dictionary, `implementation` seems invalid (disposed?)
and that's where the Context handle comes from (not from the object instance
`this` on which the weak reference is kept).

So it seems the tracking is not done on the "correct" data and can introduce a
race situation like this one, i.e. the inner `implementation` is disposed but
it's parent is not (yet). So the handle can be (and is) reused and fails to be
added.

One "hacky" solution would be to ignore it, IOW replace any existing
(half-disposed) instance with the new (fresh) one.

The "right" one would be to track the right data, the `implementation`, but
might be more invasive. Looking into it...
Comment 11 Sebastien Pouliot 2013-05-27 17:37:52 EDT
Sometime the GraphicsContext dispose of the EAGLContext (in it's Dispose
method) and sometime the EAGLContext is disposed before (e.g. when
DestroyFrameBuffer is called).

That means the previous (accessed thru the WeakReference) `implementation`,
which is a iPhoneOSGraphicsContext, has it's EAGLContext (it's Handle is 0x0).
Now `implementation.Context` is a *copy* of `contextHandle` - the original
Handle value (IntPtr).

So that makes it possible, on the native side, to reuse the same handle (it's
been freed) while the managed side still as a copy of the value
(contextHandle). That happens when the GraphicsContext has not yet been
disposed and it means it's possible to re-add the same handle value into the
dictionary.

Cleaning that `contextHandle` (when disposed) value does not work since it's
used to remove entries from the dictionary (so the problem remains).
Comment 12 Sebastien Pouliot 2013-05-27 20:38:28 EDT
The second bug seems to dispappear when we ensure the dispose order remains
identical everywhere (I'm well over 10k instances).

Part's of GraphicsContext Dispose job is to call EAGLContext.Dispose so this
ensure they get released in the "right" order (so the dictionary is kept in
sync). 

OTOH maybe I'm missing the point why only the EAGLContext was disposed and the
GraphicsContext only null'ed ?!?

diff --git a/src/OpenGLES/OpenTK_1.0/Platform/iPhoneOS/iPhoneOSGameView.cs
b/src/OpenGLES/OpenTK_1.0/Platform/iPhoneOS/iPhoneOSGameView.cs
index 7bb7d03..e757940 100644
--- a/src/OpenGLES/OpenTK_1.0/Platform/iPhoneOS/iPhoneOSGameView.cs
+++ b/src/OpenGLES/OpenTK_1.0/Platform/iPhoneOS/iPhoneOSGameView.cs
@@ -557,7 +557,7 @@ namespace OpenTK.Platform.iPhoneOS
             else
                 EAGLContext.SetCurrentContext(null);

-            EAGLContext.Dispose();
+            GraphicsContext.Dispose();
             GraphicsContext = null;
             gl = null;
         }
Comment 13 Sebastien Pouliot 2013-05-28 08:45:29 EDT
The first bug (missing finalizer), inside opentk, has been filled as
http://www.opentk.com/node/3330
Comment 14 Sebastien Pouliot 2013-05-28 10:25:56 EDT
Fixes for the two issues are now fixed in
938cf69518205d26fe4243dca1a0b6cdf8dbb46a (master)
Comment 15 Nischal 2013-06-14 03:48:34 EDT
Today I have checked this issue with following builds:

X.S 4.0.8-1
Xamarin.iOS 6.3.6-76
Mono 3.0.11

And we have run the attached project, It deployed and launched successfully on
both Simulator and device.

Changing the status to Verified.
Comment 16 narayanp 2013-06-14 04:41:57 EDT
An Update to Comment#15 

I have run this application for 15 minutes on physical device and I am not
seeing any crash.

Note You need to log in before you can comment on or make changes to this bug.