Permalink
Browse files

GraphicsContext cannot be released by finalizer

On many/most platforms, GraphicsContexts can only be released by the
thread where they are current. This means that the user must call
GraphicsContext.Dispose() or risk a resource leak.

Since we cannot release contexts on the finalizer thread, we should keep
strong references, instead of weak references, until the user explicitly
calls Dispose().

This patch fixes issues with SDL2 crashing when running the MonoGame
WindowsGL test suite.
  • Loading branch information...
1 parent 498f16c commit 3c6682e080fcfe71d25feb6d7e4395a81cd1b3a3 @thefiddler thefiddler committed Dec 16, 2013
Showing with 25 additions and 10 deletions.
  1. +25 −10 Source/OpenTK/Graphics/GraphicsContext.cs
View
35 Source/OpenTK/Graphics/GraphicsContext.cs
@@ -48,12 +48,20 @@ public sealed class GraphicsContext : IGraphicsContext, IGraphicsContextInternal
// context - we'll not destroy it manually.
readonly bool IsExternal;
bool check_errors = true;
+ // Cache for the context handle. We need this for RemoveContext()
+ // in case the user does not call Dispose(). When this happens,
+ // RemoveContext() is called by the finalizer, in which case
+ // the IGraphicsContext implementation may already be null
+ // (hence we cannot call implementation.Context to retrieve
+ // the handle.)
+ ContextHandle handle_cached;
static bool share_contexts = true;
static bool direct_rendering = true;
readonly static object SyncRoot = new object();
- // Maps OS-specific context handles to GraphicsContext weak references.
- readonly static Dictionary<ContextHandle, WeakReference> available_contexts = new Dictionary<ContextHandle, WeakReference>();
+ // Maps OS-specific context handles to GraphicsContext instances.
+ readonly static Dictionary<ContextHandle, IGraphicsContext> available_contexts =
+ new Dictionary<ContextHandle, IGraphicsContext>();
#endregion
@@ -143,6 +151,7 @@ public GraphicsContext(GraphicsMode mode, IWindowInfo window, int major, int min
}
implementation = factory.CreateGLContext(mode, window, shareContext, direct_rendering, major, minor, flags);
+ handle_cached = ((IGraphicsContextInternal)implementation).Context;
}
AddContext(this);
@@ -176,6 +185,7 @@ public GraphicsContext(ContextHandle handle, IWindowInfo window)
/// <param name="flags">A bitwise combination of <see cref="GraphicsContextFlags"/> that describe this context.</param>
/// <exception cref="GraphicsContextException">Occurs if handle is identical to a context already registered with OpenTK.</exception>
public GraphicsContext(ContextHandle handle, IWindowInfo window, IGraphicsContext shareContext, int major, int minor, GraphicsContextFlags flags)
+ : this(handle)
{
lock (SyncRoot)
{
@@ -198,7 +208,6 @@ public GraphicsContext(ContextHandle handle, IWindowInfo window, IGraphicsContex
}
}
- AddContext(this);
(this as IGraphicsContextInternal).LoadAll();
}
}
@@ -245,13 +254,13 @@ static void AddContext(IGraphicsContextInternal context)
ContextHandle ctx = context.Context;
if (!available_contexts.ContainsKey(ctx))
{
- available_contexts.Add(ctx, new WeakReference(context));
+ available_contexts.Add(ctx, (IGraphicsContext)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);
+ available_contexts[ctx] = (IGraphicsContext)context;
}
}
@@ -273,13 +282,12 @@ static IGraphicsContext FindSharedContext()
if (GraphicsContext.ShareContexts)
{
// A small hack to create a shared context with the first available context.
- foreach (WeakReference r in GraphicsContext.available_contexts.Values)
+ foreach (IGraphicsContext target in GraphicsContext.available_contexts.Values)
{
// Fix for bug 1874: if a GraphicsContext gets finalized
// (but not disposed), it won't be removed from available_contexts
// making this return null even if another valid context exists.
// The workaround is to simply ignore null targets.
- IGraphicsContext target = r.Target as IGraphicsContext;
if (target != null)
return target;
}
@@ -364,7 +372,7 @@ public static IGraphicsContext CurrentContext
{
ContextHandle handle = GetCurrentContext();
if (handle.Handle != IntPtr.Zero)
- return (GraphicsContext)available_contexts[handle].Target;
+ return (IGraphicsContext)available_contexts[handle];
}
return null;
}
@@ -532,7 +540,14 @@ IGraphicsContext IGraphicsContextInternal.Implementation
/// </summary>
ContextHandle IGraphicsContextInternal.Context
{
- get { return ((IGraphicsContextInternal)implementation).Context; }
+ get
+ {
+ if (implementation != null)
+ {
+ handle_cached = ((IGraphicsContextInternal)implementation).Context;
+ }
+ return handle_cached;
+ }
}
/// <summary>
@@ -599,7 +614,7 @@ void Dispose(bool manual)
// Note: we cannot dispose the implementation
// from a different thread. See wglDeleteContext.
// This is also known to crash GLX implementations.
- if (manual && !IsExternal)
+ if (manual)
{
Debug.Print("Disposing context {0}.", (this as IGraphicsContextInternal).Context.ToString());
if (implementation != null)

0 comments on commit 3c6682e

Please sign in to comment.