8000 Finalizer raises FinalizationException when it sees an object from pr… · pythonnet/pythonnet@1897d1b · GitHub
[go: up one dir, main page]
Skip to content

Commit 1897d1b

Browse files
lostmsulostmsu
committed
Finalizer raises FinalizationException when it sees an object from previous run.
1 parent 62e2fb4 commit 1897d1b

File tree

5 files changed

+131
-39
lines changed

5 files changed

+131
-39
lines changed

‎src/embed_tests/pyinitialize.cs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,10 @@ public static void LoadSpecificArgs()
4040
{
4141
using (var argv = new PyList(Runtime.Runtime.PySys_GetObject("argv")))
4242
{
43-
Assert.AreEqual(args[0], argv[0].ToString());
44-
Assert.AreEqual(args[1], argv[1].ToString());
43+
using var v0 = argv[0];
44+
using var v1 = argv[1];
45+
Assert.AreEqual(args[0], v0.ToString());
46+
Assert.AreEqual(args[1], v1.ToString());
4547
}
4648
}
4749
}
@@ -54,12 +56,16 @@ public void ImportClassShutdownRefcount()
5456

5557
PyObject ns = Py.Import(typeof(ImportClassShutdownRefcountClass).Namespace);
5658
PyObject cls = ns.GetAttr(nameof(ImportClassShutdownRefcountClass));
59+
BorrowedReference clsRef = cls.Reference;
60+
#pragma warning disable CS0618 // Type or member is obsolete
61+
cls.Leak();
62+
#pragma warning restore CS0618 // Type or member is obsolete
5763
ns.Dispose();
5864

59-
Assert.Less(cls.Refcount, 256);
65+
Assert.Less(Runtime.Runtime.Refcount(clsRef), 256);
6066

6167
PythonEngine.Shutdown();
62-
Assert.Greater(cls.Refcount, 0);
68+
Assert.Greater(Runtime< 1E80 /span>.Runtime.Refcount(clsRef), 0);
6369
}
6470

6571
/// <summary>

‎src/runtime/finalizer.cs

Lines changed: 67 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using System.Collections.Concurrent;
33
using System.Collections.Generic;
44
using System.ComponentModel;
5+
using System.Diagnostics;
56
using System.Linq;
67
using System.Threading;
78
using System.Threading.Tasks;
@@ -30,10 +31,12 @@ public class ErrorArgs : EventArgs
3031
[DefaultValue(DefaultThreshold)]
3132
public int Threshold { get; set; } = DefaultThreshold;
3233

34+
bool started;
35+
3336
[DefaultValue(true)]
3437
public bool Enable { get; set; } = true;
3538

36-
private ConcurrentQueue<IntPtr> _objQueue = new ConcurrentQueue<IntPtr>();
39+
private ConcurrentQueue<PendingFinalization> _objQueue = new ();
3740
private int _throttled;
3841

3942
#region FINALIZER_CHECK
@@ -79,6 +82,8 @@ internal IncorrectRefCountException(IntPtr ptr)
7982

8083
internal void ThrottledCollect()
8184
{
85+
if (!started) throw new InvalidOperationException($"{nameof(PythonEngine)} is not initialized");
86+
8287
_throttled = unchecked(this._throttled + 1);
8388
if (!Enable || _throttled < Threshold) return;
8489
_throttled = 0;
@@ -87,12 +92,13 @@ internal void ThrottledCollect()
8792

8893
internal List<IntPtr> GetCollectedObjects()
8994
{
90-
return _objQueue.ToList();
95+
return _objQueue.Select(o => o.PyObj).ToList();
9196
}
9297

93-
internal void AddFinalizedObject(ref IntPtr obj)
98+
internal void AddFinalizedObject(ref IntPtr obj, int run)
9499
{
95-
if (!Enable || obj == IntPtr.Zero)
100+
Debug.Assert(obj != IntPtr.Zero);
101+
if (!Enable)
96102
{
97103
return;
98104
}
@@ -101,14 +107,20 @@ internal void AddFinalizedObject(ref IntPtr obj)
101107
lock (_queueLock)
102108
#endif
103109
{
104-
this._objQueue.Enqueue(obj);
110+
this._objQueue.Enqueue(new PendingFinalization { PyObj = obj, RuntimeRun = run });
105111
}
106112
obj = IntPtr.Zero;
107113
}
108114

115+
internal static void Initialize()
116+
{
117+
Instance.started = true;
118+
}
119+
109120
internal static void Shutdown()
110121
{
111122
Instance.DisposeAll();
123+
Instance.started = false;
112124
}
113125

114126
private void DisposeAll()
@@ -124,36 +136,31 @@ private void DisposeAll()
124136
#if FINALIZER_CHECK
125137
ValidateRefCount();
126138
#endif
127-
IntPtr obj;
128139
Runtime.PyErr_Fetch(out var errType, out var errVal, out var traceback);
129140

141+
int run = Runtime.GetRun();
142+
130143
try
131144
{
132145
while (!_objQueue.IsEmpty)
133146
{
134-
if (!_objQueue.TryDequeue(out obj))
147+
if (!_objQueue.TryDequeue(out var obj))
148+
continue;
149+
150+
if (obj.RuntimeRun != run)
151+
{
152+
HandleFinalizationException(obj.PyObj, new RuntimeShutdownException(obj.PyObj));
135153
continue;
154+
}
136155

137-
Runtime.XDecref(obj);
156+
Runtime.XDecref(obj.PyObj);
138157
try
139158
{
140159
Runtime.CheckExceptionOccurred();
141160
}
142161
catch (Exception e)
143162
{
144-
var errorArgs = new ErrorArgs
145-
{
146-
Error = e,
147-
};
148-
149-
ErrorHandler?.Invoke(this, errorArgs);
150-
151-
if (!errorArgs.Handled)
152-
{
153-
throw new FinalizationException(
154-
"Python object finalization failed",
155-
disposable: obj, innerException: e);
156-
}
163+
HandleFinalizationException(obj.PyObj, e);
157164
}
158165
}
159166
}
@@ -166,6 +173,23 @@ private void DisposeAll()
166173
}
167174
}
168175

176+
void HandleFinalizationException(IntPtr obj, Exception cause)
177+
{
178+
var errorArgs = new ErrorArgs
179+
{
180+
Error = cause,
181+
};
182+
183+
ErrorHandler?.Invoke(this, errorArgs);
184+
185+
if (!errorArgs.Handled)
186+
{
187+
throw new FinalizationException(
188+
"Python object finalization failed",
189+
disposable: obj, innerException: cause);
190+
}
191+
}
192+
169193
#if FINALIZER_CHECK
170194
private void ValidateRefCount()
171195
{
@@ -235,6 +259,12 @@ private void ValidateRefCount()
235259
#endif
236260
}
237261

262+
struct PendingFinalization
263+
{
264+
public IntPtr PyObj;
265+
public int RuntimeRun;
266+
}
267+
238268
public class FinalizationException : Exception
239269
{
240270
public IntPtr Handle { get; }
@@ -259,5 +289,21 @@ public FinalizationException(string message, IntPtr disposable, Exception innerE
259289
if (disposable == IntPtr.Zero) throw new ArgumentNullException(nameof(disposable));
260290
this.Handle = disposable;
261291
}
292+
293+
protected FinalizationException(string message, IntPtr disposable)
294+
: base(message)
295+
{
296+
if (disposable == IntPtr.Zero) throw new ArgumentNullException(nameof(disposable));
297+
this.Handle = disposable;
298+
}
299+
}
300+
301+
public class RuntimeShutdownException : FinalizationException
302+
{
303+
public RuntimeShutdownException(IntPtr disposable)
304+
: base("Python runtime was shut down after this object was created." +
305+
" It is an error to attempt to dispose or to continue using it even after restarting the runtime.", disposable)
306+
{
307+
}
262308
}
263309
}

‎src/runtime/pybuffer.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,9 +238,10 @@ private void Dispose(bool disposing)
238238
{
239239
Debug.Assert(!disposedValue);
240240

241-
_exporter.CheckRun();
242-
243-
Finalizer.Instance.AddFinalizedObject(ref _view.obj);
241+
if (_view.obj != IntPtr.Zero)
242+
{
243+
Finalizer.Instance.AddFinalizedObject(ref _view.obj, _exporter.run);
244+
}
244245
}
245246

246247
/// <summary>

‎src/runtime/pyobject.cs

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
using System;
2-
using System.Collections;
32
using System.Collections.Generic;
43
using System.Diagnostics;
54
using System.Dynamic;
65
using System.Linq;
76
using System.Linq.Expressions;
7+
using System.Threading;
88

99
namespace Python.Runtime
1010
{
@@ -27,7 +27,7 @@ public partial class PyObject : DynamicObject, IDisposable
2727
#endif
2828

2929
protected internal IntPtr obj = IntPtr.Zero;
30-
readonly int run = Runtime.GetRun();
30+
internal readonly int run = Runtime.GetRun();
3131

3232
public static PyObject None => new PyObject(new BorrowedReference(Runtime.PyNone));
3333
internal BorrowedReference Reference => new BorrowedReference(this.obj);
@@ -96,13 +96,19 @@ internal PyObject(in StolenReference reference)
9696
// when the managed wrapper is garbage-collected.
9797
~PyObject()
9898
{
99-
Debug.Assert(obj != IntPtr.Zero);
99+
Debug.Assert(obj != IntPtr.Zero || this.GetType() != typeof(PyObject));
100+
101+
if (obj != IntPtr.Zero)
102+
{
100103

101104
#if TRACE_ALLOC
102-
CheckRun();
105+
CheckRun();
103106
#endif
104107

105-
Finalizer.Instance.AddFinalizedObject(ref obj);
108+
Interlocked.Increment(ref Runtime._collected);
109+
110+
Finalizer.Instance.AddFinalizedObject(ref obj, run);
111+
}
106112

107113
Dispose(false);
108114
}
@@ -231,12 +237,18 @@ public void Dispose()
231237
Dispose(true);
232238
}
233239

240+
[Obsolete("Test use only")]
241+
internal void Leak()
242+
{
243+
Debug.Assert(obj != IntPtr.Zero);
244+
GC.SuppressFinalize(this);
245+
obj = IntPtr.Zero;
246+
}
247+
234248
internal void CheckRun()
235249
{
236250
if (run != Runtime.GetRun())
237-
throw new InvalidOperationException(
238-
"PythonEngine was shut down after this object was created." +
239-
" It is an error to attempt to dispose or to continue using it.");
251+
throw new RuntimeShutdownException(obj);
240252
}
241253

242254
internal BorrowedReference GetPythonTypeReference()

‎src/runtime/runtime.cs

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ internal static void Initialize(bool initSigs = false, ShutdownMode mode = Shutd
155155
MainManagedThreadId = Thread.CurrentThread.ManagedThreadId;
156156

157157
IsFinalizing = false;
158+
Finalizer.Initialize();
158159
InternString.Initialize();
159160

160161
InitPyMembers();
@@ -378,6 +379,8 @@ internal static void Shutdown(ShutdownMode mode)
378379
DisposeLazyModule(inspect);
379380
PyObjectConversions.Reset();
380381

382+
bool everythingSeemsCollected = TryCollectingGarbage();
383+
Debug.Assert(everythingSeemsCollected);
381384
Finalizer.Shutdown();
382385
InternString.Shutdown();
383386

@@ -423,6 +426,26 @@ internal static void Shutdown(ShutdownMode mode)
423426
}
424427
}
425428

429+
const int MaxCollectRetriesOnShutdown = 20;
430+
internal static int _collected;
431+
static bool TryCollectingGarbage()
432+
{
433+
for (int attempt = 0; attempt < MaxCollectRetriesOnShutdown; attempt++)
434+
{
435+
Interlocked.Exchange(ref _collected, 0);
436+
nint pyCollected = 0;
437+
for (int i = 0; i < 2; i++)
438+
{
439+
GC.Collect();
440+
GC.WaitForPendingFinalizers();
441+
pyCollected += PyGC_Collect();
442+
}
443+
if (Volatile.Read(ref _collected) == 0 && pyCollected == 0)
444+
return true;
445+
}
446+
return false;
447+
}
448+
426449
internal static void Shutdown()
427450
{
428451
var mode = ShutdownMode;
@@ -818,6 +841,10 @@ internal static unsafe long Refcount(IntPtr op)
818841
return *p;
819842
}
820843

844+
[Pure]
845+
internal static long Refcount(BorrowedReference op)
846+
=> Refcount(op.DangerousGetAddress());
847+
821848
/// <summary>
822849
/// Call specified function, and handle PythonDLL-related failures.
823850
/// </summary>
@@ -2212,7 +2239,7 @@ internal static int PyException_SetTraceback(BorrowedReference ex, BorrowedRefer
22122239

22132240

22142241

2215-
internal static IntPtr PyGC_Collect() => Delegates.PyGC_Collect();
2242+
internal static nint PyGC_Collect() => Delegates.PyGC_Collect();
22162243

22172244
internal static IntPtr _Py_AS_GC(BorrowedReference ob)
22182245
{
@@ -2592,7 +2619,7 @@ static Delegates()
25922619
PyErr_Print = (delegate* unmanaged[Cdecl]<void>)GetFunctionByName(nameof(PyErr_Print), GetUnmanagedDll(_PythonDll));
25932620
PyCell_Get = (delegate* unmanaged[Cdecl]<BorrowedReference, NewReference>)GetFunctionByName(nameof(PyCell_Get), GetUnmanagedDll(_PythonDll));
25942621
PyCell_Set = (delegate* unmanaged[Cdecl]<BorrowedReference, IntPtr, int>)GetFunctionByName(nameof(PyCell_Set), GetUnmanagedDll(_PythonDll));
2595-
PyGC_Collect = (delegate* unmanaged[Cdecl]<IntPtr>)GetFunctionByName(nameof(PyGC_Collect), GetUnmanagedDll(_PythonDll));
2622+
PyGC_Collect = (delegate* unmanaged[Cdecl]<nint>)GetFunctionByName(nameof(PyGC_Collect), GetUnmanagedDll(_PythonDll));
25962623
PyCapsule_New = (delegate* unmanaged[Cdecl]<IntPtr, IntPtr, IntPtr, NewReference>)GetFunctionByName(nameof(PyCapsule_New), GetUnmanagedDll(_PythonDll));
25972624
PyCapsule_GetPointer = (delegate* unmanaged[Cdecl]<BorrowedReference, IntPtr, IntPtr>)GetFunctionByName(nameof(PyCapsule_GetPointer), GetUnmanagedDll(_PythonDll));
25982625
PyCapsule_SetPointer = (delegate* unmanaged[Cdecl]<BorrowedReference, IntPtr, int>)GetFunctionByName(nameof(PyCapsule_SetPointer), GetUnmanagedDll(_PythonDll));
@@ -2871,7 +2898,7 @@ static Delegates()
28712898
internal static delegate* unmanaged[Cdecl]<void> PyErr_Print { get; }
28722899
internal static delegate* unmanaged[Cdecl]<BorrowedReference, NewReference> PyCell_Get { get; }
28732900
internal static delegate* unmanaged[Cdecl]<BorrowedReference, IntPtr, int> PyCell_Set { get; }
2874-
internal static delegate* unmanaged[Cdecl]<IntPtr> PyGC_Collect { get; }
2901+
internal static delegate* unmanaged[Cdecl]<nint> PyGC_Collect { get; }
28752902
internal static delegate* unmanaged[Cdecl]<IntPtr, IntPtr, IntPtr, NewReference> PyCapsule_New { get; }
28762903
internal static delegate* unmanaged[Cdecl]<BorrowedReference, IntPtr, IntPtr> PyCapsule_GetPointer { get; }
28772904
internal static delegate* unmanaged[Cdecl]<BorrowedReference, IntPtr, int> PyCapsule_SetPointer { get; }

0 commit comments

Comments
 (0)
0