From b7af883cff5db971bba1afe60ffe2c3fa276f194 Mon Sep 17 00:00:00 2001
From: "Stefanos A." <stapostol@gmail.com>
Date: Thu, 21 Nov 2013 09:34:06 +0100
Subject: [PATCH] Fix exceptions on reused OpenGL context handles

Implementations may reuse OpenGL context handles that have been
destroyed. If a context is finalized but not Disposed, then OpenTK may
keep a reference to the old context handle, causing a crash when the
same handle is returned for a new context. To fix that, new context
handles will now replace old handles in case of a clash.
---
 Source/OpenTK/Graphics/GraphicsContext.cs | 74 ++++++++++++++---------
 1 file changed, 45 insertions(+), 29 deletions(-)

diff --git a/Source/OpenTK/Graphics/GraphicsContext.cs b/Source/OpenTK/Graphics/GraphicsContext.cs
index bcd9dc49..bcda8525 100644
--- a/Source/OpenTK/Graphics/GraphicsContext.cs
+++ b/Source/OpenTK/Graphics/GraphicsContext.cs
@@ -66,7 +66,7 @@ namespace OpenTK.Graphics
 
             lock (SyncRoot)
             {
-                available_contexts.Add((implementation as IGraphicsContextInternal).Context, new WeakReference(this));
+                AddContext(this);
             }
         }
         
@@ -145,7 +145,7 @@ namespace OpenTK.Graphics
                         implementation = factory.CreateGLContext(mode, window, shareContext, direct_rendering, major, minor, flags);
                     }
 
-                    available_contexts.Add((this as IGraphicsContextInternal).Context, new WeakReference(this));
+                    AddContext(this);
                 }
                 finally
                 {
@@ -198,8 +198,7 @@ namespace OpenTK.Graphics
                     }
                 }
 
-                available_contexts.Add((implementation as IGraphicsContextInternal).Context, new WeakReference(this));
-
+                AddContext(this);
                 (this as IGraphicsContextInternal).LoadAll();
             }
         }
@@ -241,6 +240,34 @@ namespace OpenTK.Graphics
 
         #region Private Members
 
+        static void AddContext(IGraphicsContextInternal context)
+        {
+            ContextHandle ctx = context.Context;
+            if (!available_contexts.ContainsKey(ctx))
+            {
+                available_contexts.Add(ctx, new WeakReference(context));
+            }
+            else
+            {
+                Debug.Print("A GraphicsContext with handle {0} already exists.", ctx);
+                Debug.Print("Did you forget to call Dispose()?");
+                available_contexts[ctx] = new WeakReference(context);
+            }
+        }
+
+        static void RemoveContext(IGraphicsContextInternal context)
+        {
+            ContextHandle ctx = context.Context;
+            if (available_contexts.ContainsKey(ctx))
+            {
+                available_contexts.Remove(ctx);
+            }
+            else
+            {
+                Debug.Print("Tried to remove non-existent GraphicsContext handle {0}. Call Dispose() to avoid this error.", ctx);
+            }
+        }
+
         static IGraphicsContext FindSharedContext()
         {
             if (GraphicsContext.ShareContexts)
@@ -394,29 +421,6 @@ namespace OpenTK.Graphics
             get { return check_errors; }
             set { check_errors = value; }
         }
-        /// <summary>
-        /// Creates an OpenGL context with the specified direct/indirect rendering mode and sharing state with the
-        /// specified IGraphicsContext.
-        /// </summary>
-        /// <param name="direct">Set to true for direct rendering or false otherwise.</param>
-        /// <param name="source">The source IGraphicsContext to share state from.</param>.
-        /// <remarks>
-        /// <para>
-        /// Direct rendering is the default rendering mode for OpenTK, since it can provide higher performance
-        /// in some circumastances.
-        /// </para>
-        /// <para>
-        /// The 'direct' parameter is a hint, and will ignored if the specified mode is not supported (e.g. setting
-        /// indirect rendering on Windows platforms).
-        /// </para>
-        /// </remarks>
-        void CreateContext(bool direct, IGraphicsContext source)
-        {
-            lock (SyncRoot)
-            {
-                available_contexts.Add((this as IGraphicsContextInternal).Context, new WeakReference(this));
-            }
-        }
 
         /// <summary>
         /// Swaps buffers on a context. This presents the rendered scene to the user.
@@ -569,21 +573,33 @@ namespace OpenTK.Graphics
         {
             if (!IsDisposed)
             {
-                Debug.Print("Disposing context {0}.", (this as IGraphicsContextInternal).Context.ToString());
                 lock (SyncRoot)
                 {
-                    available_contexts.Remove((this as IGraphicsContextInternal).Context);
+                    RemoveContext(this);
                 }
 
+                // Note: we cannot dispose the implementation
+                // from a different thread. See wglDeleteContext.
+                // This is also known to crash GLX implementations.
                 if (manual && !IsExternal)
                 {
+                    Debug.Print("Disposing context {0}.", (this as IGraphicsContextInternal).Context.ToString());
                     if (implementation != null)
                         implementation.Dispose();
                 }
+                else
+                {
+                    Debug.WriteLine("GraphicsContext leaked, did you forget to call Dispose()?");
+                }
                 IsDisposed = true;
             }
         }
 
+        ~GraphicsContext()
+        {
+            Dispose(false);
+        }
+
         #endregion
     }
 }