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

Merge pull request #1313 from losttech/bugs/1309 · sdpython/pythonnet@6bc85ff · GitHub

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

Repository navigation

Commit 6bc85ff

Browse files
filmor
authored
Merge pull request pythonnet#1313 from losttech/bugs/1309
Fixed CollectBasicObject test
2 parents c23958e + ca96444 commit 6bc85ff

10 files changed

Lines changed: 132 additions & 211 deletions

File tree

‎src/embed_tests/TestDomainReload.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ from Python.EmbeddingTest.Domain import MyClass
191191
def test_obj_call():
192192
obj = MyClass()
193193
obj.Method()
194-
obj.StaticMethod()
194+
MyClass.StaticMethod()
195195
obj.Property = 1
196196
obj.Field = 10
197197
@@ -288,7 +288,7 @@ void ExecTest()
288288

289289
GC.Collect();
290290
GC.WaitForPendingFinalizers(); // <- this will put former `num` into Finalizer queue
291-
Finalizer.Instance.Collect(forceDispose: true);
291+
Finalizer.Instance.Collect();
292292
// ^- this will call PyObject.Dispose, which will call XDecref on `num.Handle`,
293293
// but Python interpreter from "run" 1 is long gone, so it will corrupt memory instead.
294294
Assert.False(numRef.IsAlive);
@@ -333,7 +333,7 @@ void ExecTest()
333333
PythonEngine.Initialize(); // <- "run" 2 starts
334334
GC.Collect();
335335
GC.WaitForPendingFinalizers();
336-
Finalizer.Instance.Collect(forceDispose: true);
336+
Finalizer.Instance.Collect();
337337
Assert.False(objRef.IsAlive);
338338
}
339339
finally

‎src/embed_tests/TestFinalizer.cs‎

Lines changed: 33 additions & 116 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);
@@ -64,11 +52,7 @@ public void CollectBasicObject()
6452
Assert.IsFalse(called, "The event handler was called before it was installed");
6553
Finalizer.Instance.CollectOnce += handler;
6654

67-
WeakReference shortWeak;
68-
WeakReference longWeak;
69-
{
70-
MakeAGarbage(out shortWeak, out longWeak);
71-
}
55+
IntPtr pyObj = MakeAGarbage(out var shortWeak, out var longWeak);
7256
FullGCCollect();
7357
// The object has been resurrected
7458
Warn.If(
@@ -86,7 +70,7 @@ public void CollectBasicObject()
8670
var garbage = Finalizer.Instance.GetCollectedObjects();
8771
Assert.NotZero(garbage.Count, "There should still be garbage around");
8872
Warn.Unless(
89-
garbage.Any(T => ReferenceEquals(T.Target, longWeak.Target)),
73+
garbage.Contains(pyObj),
9074
$"The {nameof(longWeak)} reference doesn't show up in the garbage list",
9175
garbage
9276
);
@@ -104,33 +88,45 @@ public void CollectBasicObject()
10488
}
10589

10690
[Test]
107-
[Ignore("Ignore temporarily")]
91+
[Obsolete("GC tests are not guaranteed")]
10892
public void CollectOnShutdown()
10993
{
11094
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-
}
95+
FullGCCollect();
11796
Assert.IsFalse(shortWeak.IsAlive);
118-
garbage = Finalizer.Instance.GetCollectedObjects();
97+
List<IntPtr> garbage = Finalizer.Instance.GetCollectedObjects();
11998
Assert.IsNotEmpty(garbage, "The garbage object should be collected");
120-
Assert.IsTrue(garbage.Any(r => ReferenceEquals(r.Target, longWeak.Target)),
99+
Assert.IsTrue(garbage.Contains(op),
121100
"Garbage should contains the collected object");
122101

123102
PythonEngine.Shutdown();
124103
garbage = Finalizer.Instance.GetCollectedObjects();
125104
Assert.IsEmpty(garbage);
126105
}
127106

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

136132
private static long CompareWithFinalizerOn(PyObject pyCollect, bool enbale)
@@ -191,62 +187,6 @@ public void SimpleTestMemory()
191187
}
192188
}
193189

194-
class MyPyObject : PyObject
195-
{
196-
public MyPyObject(IntPtr op) : base(op)
197-
{
198-
}
199-
200-
protected override void Dispose(bool disposing)
201-
{
202-
base.Dispose(disposing);
203-
GC.SuppressFinalize(this);
204-
throw new Exception("MyPyObject");
205-
}
206-
internal static void CreateMyPyObject(IntPtr op)
207-
{
208-
Runtime.Runtime.XIncref(op);
209-
new MyPyObject(op);
210-
}
211-
}
212-
213-
[Test]
214-
public void ErrorHandling()
215-
{
216-
bool called = false;
217-
var errorMessage = "";
218-
EventHandler<Finalizer.ErrorArgs> handleFunc = (sender, args) =>
219-
{
220-
called = true;
221-
errorMessage = args.Error.Message;
222-
};
223-
Finalizer.Instance.Threshold = 1;
224-
Finalizer.Instance.ErrorHandler += handleFunc;
225-
try
226-
{
227-
WeakReference shortWeak;
228-
WeakReference longWeak;
229-
{
230-
MakeAGarbage(out shortWeak, out longWeak);
231-
var obj = (PyLong)longWeak.Target;
232-
IntPtr handle = obj.Handle;
233-
shortWeak = null;
234-
longWeak = null;
235-
MyPyObject.CreateMyPyObject(handle);
236-
obj.Dispose();
237-
obj = null;
238-
}
239-
FullGCCollect();
240-
Finalizer.Instance.Collect();
241-
Assert.IsTrue(called);
242-
}
243-
finally
244-
{
245-
Finalizer.Instance.ErrorHandler -= handleFunc;
246-
}
247-
Assert.AreEqual(errorMessage, "MyPyObject");
248-
}
249-
250190
[Test]
251191
public void ValidateRefCount()
252192
{
@@ -279,36 +219,13 @@ public void ValidateRefCount()
279219
}
280220
}
281221

222+
[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)] // ensure lack of references to s1 and s2
282223
private static IntPtr CreateStringGarbage()
283224
{
284225
PyString s1 = new PyString("test_string");
285226
// s2 steal a reference from s1
286227
PyString s2 = new PyString(s1.Handle);
287228
return s1.Handle;
288229
}
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-
}
313230
}
314231
}

‎src/runtime/debughelper.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,5 +137,12 @@ public static void PrintHexBytes(byte[] bytes)
137137
Console.WriteLine();
138138
}
139139
}
140+
141+
[Conditional("DEBUG")]
142+
public static void AssertHasReferences(IntPtr obj)
143+
{
144+
long refcount = Runtime.Refcount(obj);
145+
Debug.Assert(refcount > 0, "Object refcount is 0 or less");
146+
}
140147
}
141148
}

‎src/runtime/delegatemanager.cs‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ A possible alternate strategy would be to create custom subclasses
181181
too "special" for this to work. It would be more work, so for now
182182
the 80/20 rule applies :) */
183183

184-
public class Dispatcher : IPyDisposable
184+
public class Dispatcher
185185
{
186186
public IntPtr target;
187187
public Type dtype;
@@ -202,7 +202,7 @@ public Dispatcher(IntPtr target, Type dtype)
202202
return;
203203
}
204204
_finalized = true;
205-
Finalizer.Instance.AddFinalizedObject(this);
205+
Finalizer.Instance.AddFinalizedObject(ref target);
206206
}
207207

208208
public void Dispose()
@@ -276,11 +276,6 @@ public object TrueDispatch(ArrayList args)
276276
Runtime.XDecref(op);
277277
return result;
278278
}
279-
280-
public IntPtr[] GetTrackedHandles()
281-
{
282-
return new IntPtr[] { target };
283-
}
284279
}
285280

286281

0 commit comments

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