FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fixed CollectBasicObject by ensuring MakeAGarbage is not keeping temp… · sdpython/pythonnet@d888244 · GitHub

This repository was archived by the owner on Jul 22, 2023. It is now read-only.
  1. sdpython
  2. pythonnet

Repository navigation

Commit d888244

Browse files
lostmsu
committed
fixed CollectBasicObject by ensuring MakeAGarbage is not keeping temp object alive
1 parent ce76f2e commit d888244

3 files changed

Lines changed: 46 additions & 55 deletions

File tree

‎src/embed_tests/TestFinalizer.cs‎

Lines changed: 32 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
using Python.Runtime;
33
using System;
44
using System.Collections.Generic;
5-
using System.ComponentModel;
65
using System.Diagnostics;
76
using System.Linq;
7+
using System.Runtime.CompilerServices;
88
using System.Threading;
99

1010
namespace Python.EmbeddingTest
@@ -28,26 +28,14 @@ public void TearDown()
2828
PythonEngine.Shutdown();
2929
}
3030

31-
private static bool FullGCCollect()
31+
private static void FullGCCollect()
3232
{
33-
GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced);
34-
try
35-
{
36-
return GC.WaitForFullGCComplete() == GCNotificationStatus.Succeeded;
37-
}
38-
catch (NotImplementedException)
39-
{
40-
// Some clr runtime didn't implement GC.WaitForFullGCComplete yet.
41-
return false;
42-
}
43-
finally
44-
{
45-
GC.WaitForPendingFinalizers();
46-
}
33+
GC.Collect();
34+
GC.WaitForPendingFinalizers();
4735
}
4836

4937
[Test]
50-
[Ignore("Ignore temporarily")]
38+
[Obsolete("GC tests are not guaranteed")]
5139
public void CollectBasicObject()
5240
{
5341
Assert.IsTrue(Finalizer.Instance.Enable);
@@ -104,18 +92,13 @@ public void CollectBasicObject()
10492
}
10593

10694
[Test]
107-
[Ignore("Ignore temporarily")]
95+
[Obsolete("GC tests are not guaranteed")]
10896
public void CollectOnShutdown()
10997
{
11098
IntPtr op = MakeAGarbage(out var shortWeak, out var longWeak);
111-
int hash = shortWeak.Target.GetHashCode();
112-
List<WeakReference> garbage;
113-
if (!FullGCCollect())
114-
{
115-
Assert.IsTrue(WaitForCollected(op, hash, 10000));
116-
}
99+
FullGCCollect();
117100
Assert.IsFalse(shortWeak.IsAlive);
118-
garbage = Finalizer.Instance.GetCollectedObjects();
101+
List<WeakReference> garbage = Finalizer.Instance.GetCollectedObjects();
119102
Assert.IsNotEmpty(garbage, "The garbage object should be collected");
120103
Assert.IsTrue(garbage.Any(r => ReferenceEquals(r.Target, longWeak.Target)),
121104
"Garbage should contains the collected object");
@@ -125,12 +108,29 @@ public void CollectOnShutdown()
125108
Assert.IsEmpty(garbage);
126109
}
127110

111+
[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)] // ensure lack of references to obj
112+
[Obsolete("GC tests are not guaranteed")]
128113
private static IntPtr MakeAGarbage(out WeakReference shortWeak, out WeakReference longWeak)
129114
{
130-
PyLong obj = new PyLong(1024);
131-
shortWeak = new WeakReference(obj);
132-
longWeak = new WeakReference(obj, true);
133-
return obj.Handle;
115+
IntPtr handle = IntPtr.Zero;
116+
WeakReference @short = null, @long = null;
117+
// must create Python object in the thread where we have GIL
118+
IntPtr val = PyLong.FromLong(1024);
119+
// must create temp object in a different thread to ensure it is not present
120+
// when conservatively scanning stack for GC roots.
121+
// see https://xamarin.github.io/bugzilla-archives/17/17593/bug.html
122+
var garbageGen = new Thread(() =>
123+
{
124+
var obj = new PyObject(val, skipCollect: true);
125+
@short = new WeakReference(obj);
126+
@long = new WeakReference(obj, true);
127+
handle = obj.Handle;
128+
});
129+
garbageGen.Start();
130+
Assert.IsTrue(garbageGen.Join(TimeSpan.FromSeconds(5)), "Garbage creation timed out");
131+
shortWeak = @short;
132+
longWeak = @long;
133+
return handle;
134134
}
135135

136136
private static long CompareWithFinalizerOn(PyObject pyCollect, bool enbale)
@@ -211,6 +211,7 @@ internal static void CreateMyPyObject(IntPtr op)
211211
}
212212

213213
[Test]
214+
[Obsolete("GC tests are not guaranteed")]
214215
public void ErrorHandling()
215216
{
216217
bool called = false;
@@ -228,7 +229,7 @@ public void ErrorHandling()
228229
WeakReference longWeak;
229230
{
230231
MakeAGarbage(out shortWeak, out longWeak);
231-
var obj = (PyLong)longWeak.Target;
232+
var obj = (PyObject)longWeak.Target;
232233
IntPtr handle = obj.Handle;
233234
shortWeak = null;
234235
longWeak = null;
@@ -279,36 +280,13 @@ public void ValidateRefCount()
279280
}
280281
}
281282

283+
[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)] // ensure lack of references to s1 and s2
282284
private static IntPtr CreateStringGarbage()
283285
{
284286
PyString s1 = new PyString("test_string");
285287
// s2 steal a reference from s1
286288
PyString s2 = new PyString(s1.Handle);
287289
return s1.Handle;
288290
}
289-
290-
private static bool WaitForCollected(IntPtr op, int hash, int milliseconds)
291-
{
292-
var stopwatch = Stopwatch.StartNew();
293-
do
294-
{
295-
var garbage = Finalizer.Instance.GetCollectedObjects();
296-
foreach (var item in garbage)
297-
{
298-
// The validation is not 100% precise,
299-
// but it's rare that two conditions satisfied but they're still not the same object.
300-
if (item.Target.GetHashCode() != hash)
301-
{
302-
continue;
303-
}
304-
var obj = (IPyDisposable)item.Target;
305-
if (obj.GetTrackedHandles().Contains(op))
306-
{
307-
return true;
308-
}
309-
}
310-
} while (stopwatch.ElapsedMilliseconds < milliseconds);
311-
return false;
312-
}
313291
}
314292
}

‎src/runtime/pylong.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public PyLong(uint value) : base(FromInt((int)value))
7474
}
7575

7676

77-
private static IntPtr FromLong(long value)
77+
internal static IntPtr FromLong(long value)
7878
{
7979
IntPtr val = Runtime.PyLong_FromLongLong(value);
8080
PythonException.ThrowIfIsNull(val);

‎src/runtime/pyobject.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,19 @@ public PyObject(IntPtr ptr)
5454
#endif
5555
}
5656

57+
[Obsolete("for testing purposes only")]
58+
internal PyObject(IntPtr ptr, bool skipCollect)
59+
{
60+
if (ptr == IntPtr.Zero) throw new ArgumentNullException(nameof(ptr));
61+
62+
obj = ptr;
63+
if (!skipCollect)
64+
Finalizer.Instance.ThrottledCollect();
65+
#if TRACE_ALLOC
66+
Traceback = new StackTrace(1);
67+
#endif
68+
}
69+
5770
/// <summary>
5871
/// Creates new <see cref="PyObject"/> pointing to the same object as
5972
/// the <paramref name="reference"/>. Increments refcount, allowing <see cref="PyObject"/>

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL