Clean up the test and interface

pythonnet · filmor · Apr 1, 2019 · Jun 23, 2018 · Jun 24, 2018 · Jun 24, 2018
commit 799d37e51232e814294b292c4414ea6edacba924
diff --git a/src/embed_tests/TestFinalizer.cs b/src/embed_tests/TestFinalizer.cs
@@ -1,13 +1,15 @@
 using NUnit.Framework;
 using Python.Runtime;
 using System;
+using System.Linq;
 using System.Threading;
 
 namespace Python.EmbeddingTest
 {
     public class TestFinalizer
     {
         private string _PYTHONMALLOC = string.Empty;
+        private int _oldThreshold;
 
         [SetUp]
         public void SetUp()
@@ -21,13 +23,15 @@ public void SetUp()
                 _PYTHONMALLOC = string.Empty;
             }
             Environment.SetEnvironmentVariable("PYTHONMALLOC", "malloc");
+            _oldThreshold = Finalizer.Instance.Threshold;
             PythonEngine.Initialize();
             Exceptions.Clear();
         }
 
         [TearDown]
         public void TearDown()
         {
+            Finalizer.Instance.Threshold = _oldThreshold;
             PythonEngine.Shutdown();
             if (string.IsNullOrEmpty(_PYTHONMALLOC))
             {
@@ -56,41 +60,42 @@ public void CollectBasicObject()
                 Assert.GreaterOrEqual(e.ObjectCount, 1);
                 called = true;
             };
-            Finalizer.Instance.CollectOnce += handler;
-            try
-            {
-                FullGCCollect();
-                PyLong obj = new PyLong(1024);
 
-                WeakReference shortWeak = new WeakReference(obj);
-                WeakReference longWeak = new WeakReference(obj, true);
-                obj = null;
-                FullGCCollect();
-                // The object has been resurrected
-                // FIXME: Sometimes the shortWeak would get alive 
-                //Assert.IsFalse(shortWeak.IsAlive);
-                Assert.IsTrue(longWeak.IsAlive);
+            WeakReference shortWeak;
+            WeakReference longWeak;
+            {
+                MakeAGarbage(out shortWeak, out longWeak);
+            }
+            FullGCCollect();
+            // The object has been resurrected
+            Assert.IsFalse(shortWeak.IsAlive);
+            Assert.IsTrue(longWeak.IsAlive);
 
-                Assert.IsFalse(called);
+            {
                 var garbage = Finalizer.Instance.GetCollectedObjects();
                 Assert.NotZero(garbage.Count);
-                // FIXME: If make some query for garbage,
-                // the above case will failed Assert.IsFalse(shortWeak.IsAlive)
-                //Assert.IsTrue(garbage.All(T => T.IsAlive));
+                Assert.IsTrue(garbage.Any(T => ReferenceEquals(T.Target, longWeak.Target)));
+            }
 
+            Assert.IsFalse(called);
+            Finalizer.Instance.CollectOnce += handler;
+            try
+            {
                 Finalizer.Instance.CallPendingFinalizers();
-                Assert.IsTrue(called);
-
-                FullGCCollect();
-                //Assert.IsFalse(garbage.All(T => T.IsAlive));
-
-                Assert.IsNull(longWeak.Target);
             }
             finally
             {
                 Finalizer.Instance.CollectOnce -= handler;
             }
+            Assert.IsTrue(called);
+        }
 
+        private static void MakeAGarbage(out WeakReference shortWeak, out WeakReference longWeak)
+        {
+            PyLong obj = new PyLong(1024);
+            shortWeak = new WeakReference(obj);
+            longWeak = new WeakReference(obj, true);
+            obj = null;
         }
 
         private static long CompareWithFinalizerOn(PyObject pyCollect, bool enbale)
@@ -101,7 +106,7 @@ private static long CompareWithFinalizerOn(PyObject pyCollect, bool enbale)
             FullGCCollect();
             FullGCCollect();
             pyCollect.Invoke();
-            Finalizer.Instance.CallPendingFinalizers();
+            Finalizer.Instance.Collect();
             Finalizer.Instance.Enable = enbale;
 
             // Estimate unmanaged memory size
@@ -116,7 +121,7 @@ private static long CompareWithFinalizerOn(PyObject pyCollect, bool enbale)
             pyCollect.Invoke();
             if (enbale)
             {
-                Finalizer.Instance.CallPendingFinalizers();
+                Finalizer.Instance.Collect();
             }
 
             FullGCCollect();

diff --git a/src/runtime/finalizer.cs b/src/runtime/finalizer.cs
@@ -22,7 +22,8 @@ public class CollectArgs : EventArgs
 
         [UnmanagedFunctionPointer(CallingConvention.Cdecl)]
         private delegate int PedingCall(IntPtr arg);
-        private PedingCall _collectAction;
+        private readonly PedingCall _collectAction;
+
         private bool _pending = false;
         private readonly object _collectingLock = new object();
         public int Threshold { get; set; }
@@ -32,7 +33,7 @@ private Finalizer()
         {
             Enable = true;
             Threshold = 200;
-            _collectAction = OnCollect;
+            _collectAction = OnPendingCollect;
         }
 
         public void CallPendingFinalizers()
@@ -44,6 +45,14 @@ public void CallPendingFinalizers()
             Runtime.Py_MakePendingCalls();
         }
 
+        public void Collect()
+        {
+            using (var gilState = new Py.GILState())
+            {
+                DisposeAll();
+            }
+        }
+
         public List<WeakReference> GetCollectedObjects()
         {
             return _objQueue.Select(T => new WeakReference(T)).ToList();
@@ -65,7 +74,7 @@ internal void AddFinalizedObject(IDisposable obj)
             GC.ReRegisterForFinalize(obj);
             if (_objQueue.Count >= Threshold)
             {
-                Collect();
+                AddPendingCollect();
             }
         }
 
@@ -76,7 +85,7 @@ internal static void Shutdown()
             Runtime.PyErr_Clear();
         }
 
-        private void Collect()
+        private void AddPendingCollect()
         {
             lock (_collectingLock)
             {
@@ -94,19 +103,19 @@ private void Collect()
             }
         }
 
-        private int OnCollect(IntPtr arg)
+        private int OnPendingCollect(IntPtr arg)
         {
-            CollectOnce?.Invoke(this, new CollectArgs()
-            {
-                ObjectCount = _objQueue.Count
-            });
-            DisposeAll();
+            Collect();
             _pending = false;
             return 0;
         }
 
         private void DisposeAll()
         {
+            CollectOnce?.Invoke(this, new CollectArgs()
+            {
+                ObjectCount = _objQueue.Count
+            });
             IDisposable obj;
             while (_objQueue.TryDequeue(out obj))
             {