Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Moves garbage disposal to main thread and ensures it via SafeHandles. #33

Open
wants to merge 4 commits into from

3 participants

@myrup

See http://en.sfml-dev.org/forums/index.php?topic=12248.30

I considered moving the disposal to Window.Clear(), but this necessarily isn't called. Like Window.DispatchEvents() is.

Merged ObjectBase and ObjectBaseSafeHandle since last pull request.

@zsbzsb
Collaborator

I have been looking into the memory leak issue, and fixing it does not require what you have done. The problem actually lies within the context management of SFML, however I am still working on tracking down what exactly causes the issue. Sometimes memory leaks, other times not.

@LaurentGomila

What should we do with this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 68 additions and 17 deletions.
  1. +67 −17 src/Window/ObjectBase.cs
  2. +1 −0  src/Window/Window.cs
View
84 src/Window/ObjectBase.cs
@@ -9,8 +9,11 @@ namespace SFML
/// SFML object. It's meant for internal use only
/// </summary>
////////////////////////////////////////////////////////////
- public abstract class ObjectBase : IDisposable
+ public abstract class ObjectBase : SafeHandle, IDisposable
{
+ private static System.Collections.Generic.List<ObjectBase> garbageCollectedObjects = new System.Collections.Generic.List<ObjectBase>();
+ private IntPtr cPointer;
+
////////////////////////////////////////////////////////////
/// <summary>
/// Construct the object from a pointer to the C library object
@@ -18,18 +21,9 @@ public abstract class ObjectBase : IDisposable
/// <param name="cPointer">Internal pointer to the object in the C libraries</param>
////////////////////////////////////////////////////////////
public ObjectBase(IntPtr cPointer)
+ : base(IntPtr.Zero, true)
{
- myCPointer = cPointer;
- }
-
- ////////////////////////////////////////////////////////////
- /// <summary>
- /// Dispose the object
- /// </summary>
- ////////////////////////////////////////////////////////////
- ~ObjectBase()
- {
- Dispose(false);
+ this.cPointer = cPointer;
}
////////////////////////////////////////////////////////////
@@ -40,7 +34,7 @@ public ObjectBase(IntPtr cPointer)
////////////////////////////////////////////////////////////
public IntPtr CPointer
{
- get { return myCPointer; }
+ get { return cPointer; }
}
////////////////////////////////////////////////////////////
@@ -62,10 +56,10 @@ public void Dispose()
////////////////////////////////////////////////////////////
private void Dispose(bool disposing)
{
- if (myCPointer != IntPtr.Zero)
+ if (!IsInvalid)
{
Destroy(disposing);
- myCPointer = IntPtr.Zero;
+ cPointer = IntPtr.Zero;
}
}
@@ -85,9 +79,65 @@ private void Dispose(bool disposing)
////////////////////////////////////////////////////////////
protected void SetThis(IntPtr cPointer)
{
- myCPointer = cPointer;
+ if (cPointer != IntPtr.Zero && !IsInvalid)
+ throw new ArgumentException("Possible mem leak");
+ this.cPointer = cPointer;
}
- private IntPtr myCPointer = IntPtr.Zero;
+ ////////////////////////////////////////////////////////////
+ /// <summary>
+ /// Gets whether object references a C resource
+ /// (used by SafeHandle to determine whether object is truly disposed)
+ /// </summary>
+ ////////////////////////////////////////////////////////////
+ public override bool IsInvalid
+ {
+ get
+ {
+ return cPointer == IntPtr.Zero;
+ }
+ }
+
+ ////////////////////////////////////////////////////////////
+ /// <summary>
+ /// Marks the C resource for disposal
+ /// </summary>
+ ////////////////////////////////////////////////////////////
+ protected override bool ReleaseHandle()
+ {
+ //GC will wait here until all threads hit a safe point. Thus avoiding a deadlock.
+ lock (garbageCollectedObjects)
+ garbageCollectedObjects.Add(this);
+ return true;
+ }
+
+ ////////////////////////////////////////////////////////////
+ /// <summary>
+ /// Dispose garbage collected Objects on current thread
+ /// </summary>
+ ////////////////////////////////////////////////////////////
+ public static void DisposeGarbageCollectedObjects()
+ {
+ if (garbageCollectedObjects.Count > 0)
+ {
+ ObjectBase[] garbageCollectedObjectsCopy;
+ lock (garbageCollectedObjects)
+ {
+ garbageCollectedObjectsCopy = new ObjectBase[garbageCollectedObjects.Count];
+ garbageCollectedObjects.CopyTo(garbageCollectedObjectsCopy);
+ garbageCollectedObjects.Clear();
+ }
+ foreach (var garbageCollectedObject in garbageCollectedObjectsCopy)
+ {
+ try
+ {
+ garbageCollectedObject.Dispose ();
+ } catch (Exception e)
+ {
+ Console.WriteLine (e.Message + " at " + e.StackTrace);
+ }
+ }
+ }
+ }
}
}
View
1  src/Window/Window.cs
@@ -346,6 +346,7 @@ public void DispatchEvents()
Event e;
while (PollEvent(out e))
CallEventHandler(e);
+ ObjectBase.DisposeGarbageCollectedObjects();
}
////////////////////////////////////////////////////////////
Something went wrong with that request. Please try again.