Skip to content

Commit 7caa7ed

Browse files
lostmsulostmsu
authored
Merge branch 'master' into fix_kwarg_func_resolution
2 parents 1dd8725 + 72fae73 commit 7caa7ed

File tree

7 files changed

+75
-85
lines changed

7 files changed

+75
-85
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ This document follows the conventions laid out in [Keep a CHANGELOG][].
2727
- When calling C# from Python, enable passing argument of any type to a parameter of C# type `object` by wrapping it into `PyObject` instance. ([#881][i881])
2828
- Added support for kwarg parameters when calling .NET methods from Python
2929
- Changed method for finding MSBuild using vswhere
30+
- Reworked `Finalizer`. Now objects drop into its queue upon finalization, which is periodically drained when new objects are created.
3031

3132
### Fixed
3233

‎src/embed_tests/TestFinalizer.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public void CollectBasicObject()
7777
}
7878
try
7979
{
80-
Finalizer.Instance.Collect(forceDispose: false);
80+
Finalizer.Instance.Collect();
8181
}
8282
finally
8383
{

‎src/runtime/clrobject.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,13 @@ internal CLRObject(object ob, IntPtr tp)
3535
}
3636

3737

38-
internal static CLRObject GetInstance(object ob, IntPtr pyType)
38+
static CLRObject GetInstance(object ob, IntPtr pyType)
3939
{
4040
return new CLRObject(ob, pyType);
4141
}
4242

4343

44-
internal static CLRObject GetInstance(object ob)
44+
static CLRObject GetInstance(object ob)
4545
{
4646
ClassBase cc = ClassManager.GetClass(ob.GetType());
4747
return GetInstance(ob, cc.tpHandle);

‎src/runtime/finalizer.cs

Lines changed: 33 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,7 @@ public class ErrorArgs : EventArgs
2828
public bool Enable { get; set; }
2929

3030
private ConcurrentQueue<IPyDisposable> _objQueue = new ConcurrentQueue<IPyDisposable>();
31-
private bool _pending = false;
32-
private readonly object _collectingLock = new object();
33-
private Task _finalizerTask;
31+
private int _throttled;
3432

3533
#region FINALIZER_CHECK
3634

@@ -75,19 +73,16 @@ private Finalizer()
7573
Threshold = 200;
7674
}
7775

78-
public void Collect(bool forceDispose = true)
76+
[Obsolete("forceDispose parameter is unused. All objects are disposed regardless.")]
77+
public void Collect(bool forceDispose) => this.DisposeAll();
78+
public void Collect() => this.DisposeAll();
79+
80+
internal void ThrottledCollect()
7981
{
80-
if (Instance._finalizerTask != null
81-
&& !Instance._finalizerTask.IsCompleted)
82-
{
83-
var ts = PythonEngine.BeginAllowThreads();
84-
Instance._finalizerTask.Wait();
85-
PythonEngine.EndAllowThreads(ts);
86-
}
87-
else if (forceDispose)
88-
{
89-
Instance.DisposeAll();
90-
}
82+
_throttled = unchecked(this._throttled + 1);
83+
if (!Enable || _throttled < Threshold) return;
84+
_throttled = 0;
85+
this.Collect();
9186
}
9287

9388
public List<WeakReference> GetCollectedObjects()
@@ -101,62 +96,18 @@ internal void AddFinalizedObject(IPyDisposable obj)
10196
{
10297
return;
10398
}
104-
if (Runtime.Py_IsInitialized() == 0)
105-
{
106-
// XXX: Memory will leak if a PyObject finalized after Python shutdown,
107-
// for avoiding that case, user should call GC.Collect manual before shutdown.
108-
return;
109-
}
99+
110100
#if FINALIZER_CHECK
111101
lock (_queueLock)
112102
#endif
113103
{
114-
_objQueue.Enqueue(obj);
115-
}
116-
GC.ReRegisterForFinalize(obj);
117-
if (!_pending && _objQueue.Count >= Threshold)
118-
{
119-
AddPendingCollect();
104+
this._objQueue.Enqueue(obj);
120105
}
121106
}
122107

123108
internal static void Shutdown()
124109
{
125-
if (Runtime.Py_IsInitialized() == 0)
126-
{
127-
Instance._objQueue = new ConcurrentQueue<IPyDisposable>();
128-
return;
129-
}
130-
Instance.Collect(forceDispose: true);
131-
}
132-
133-
private void AddPendingCollect()
134-
{
135-
if(Monitor.TryEnter(_collectingLock))
136-
{
137-
try
138-
{
139-
if (!_pending)
140-
{
141-
_pending = true;
142-
// should already be complete but just in case
143-
_finalizerTask?.Wait();
144-
145-
_finalizerTask = Task.Factory.StartNew(() =>
146-
{
147-
using (Py.GIL())
148-
{
149-
Instance.DisposeAll();
150-
_pending = false;
151-
}
152-
});
153-
}
154-
}
155-
finally
156-
{
157-
Monitor.Exit(_collectingLock);
158-
}
159-
}
110+
Instance.DisposeAll();
160111
}
161112

162113
private void DisposeAll()
@@ -178,12 +129,18 @@ private void DisposeAll()
178129
try
179130
{
180131
obj.Dispose();
181-
Runtime.CheckExceptionOccurred();
182132
}
183133
catch (Exception e)
184134
{
185-
// We should not bother the main thread
186-
ErrorHandler?.Invoke(this, new ErrorArgs()
135+
var handler = ErrorHandler;
136+
if (handler is null)
137+
{
138+
throw new FinalizationException(
139+
"Python object finalization failed",
140+
disposable: obj, innerException: e);
141+
}
142+
143+
handler.Invoke(this, new ErrorArgs()
187144
{
188145
Error = e
189146
});
@@ -267,4 +224,15 @@ private void ValidateRefCount()
267224
}
268225
#endif
269226
}
227+
228+
public class FinalizationException : Exception
229+
{
230+
public IPyDisposable Disposable { get; }
231+
232+
public FinalizationException(string message, IPyDisposable disposable, Exception innerException)
233+
: base(message, innerException)
234+
{
235+
this.Disposable = disposable ?? throw new ArgumentNullException(nameof(disposable));
236+
}
237+
}
270238
}

‎src/runtime/pyobject.cs

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@ public class PyObject : DynamicObject, IEnumerable, IPyDisposable
3030
#endif
3131

3232
protected internal IntPtr obj = IntPtr.Zero;
33-
private bool disposed = false;
34-
private bool _finalized = false;
3533

3634
internal BorrowedReference Reference => new BorrowedReference(obj);
3735

@@ -49,6 +47,7 @@ public PyObject(IntPtr ptr)
4947
if (ptr == IntPtr.Zero) throw new ArgumentNullException(nameof(ptr));
5048

5149
obj = ptr;
50+
Finalizer.Instance.ThrottledCollect();
5251
#if TRACE_ALLOC
5352
Traceback = new StackTrace(1);
5453
#endif
@@ -64,6 +63,7 @@ internal PyObject(BorrowedReference reference)
6463
if (reference.IsNull) throw new ArgumentNullException(nameof(reference));
6564

6665
obj = Runtime.SelfIncRef(reference.DangerousGetAddress());
66+
Finalizer.Instance.ThrottledCollect();
6767
#if TRACE_ALLOC
6868
Traceback = new StackTrace(1);
6969
#endif
@@ -74,6 +74,7 @@ internal PyObject(BorrowedReference reference)
7474
[Obsolete("Please, always use PyObject(*Reference)")]
7575
protected PyObject()
7676
{
77+
Finalizer.Instance.ThrottledCollect();
7778
#if TRACE_ALLOC
7879
Traceback = new StackTrace(1);
7980
#endif
@@ -87,12 +88,6 @@ protected PyObject()
8788
{
8889
return;
8990
}
90-
if (_finalized || disposed)
91-
{
92-
return;
93-
}
94-
// Prevent a infinity loop by calling GC.WaitForPendingFinalizers
95-
_finalized = true;
9691
Finalizer.Instance.AddFinalizedObject(this);
9792
}
9893

@@ -183,17 +178,41 @@ public T As<T>()
183178
/// </remarks>
184179
protected virtual void Dispose(bool disposing)
185180
{
186-
if (!disposed)
181+
if (this.obj == IntPtr.Zero)
187182
{
188-
if (Runtime.Py_IsInitialized() > 0 && !Runtime.IsFinalizing)
183+
return;
184+
}
185+
186+
if (Runtime.Py_IsInitialized() == 0)
187+
throw new InvalidOperationException("Python runtime must be initialized");
188+
189+
if (!Runtime.IsFinalizing)
190+
{
191+
long refcount = Runtime.Refcount(this.obj);
192+
Debug.Assert(refcount > 0, "Object refcount is 0 or less");
193+
194+
if (refcount == 1)
195+
{
196+
Runtime.PyErr_Fetch(out var errType, out var errVal, out var traceback);
197+
198+
try
199+
{
200+
Runtime.XDecref(this.obj);
201+
Runtime.CheckExceptionOccurred();
202+
}
203+
finally
204+
{
205+
// Python requires finalizers to preserve exception:
206+
// https://docs.python.org/3/extending/newtypes.html#finalization-and-de-allocation
207+
Runtime.PyErr_Restore(errType, errVal, traceback);
208+
}
209+
}
210+
else
189211
{
190-
IntPtr gs = PythonEngine.AcquireLock();
191-
Runtime.XDecref(obj);
192-
obj = IntPtr.Zero;
193-
PythonEngine.ReleaseLock(gs);
212+
Runtime.XDecref(this.obj);
194213
}
195-
disposed = true;
196214
}
215+
this.obj = IntPtr.Zero;
197216
}
198217

199218
public void Dispose()

‎src/runtime/pythonengine.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ public static string PythonHome
8080
}
8181
set
8282
{
83+
// this value is null in the beginning
8384
Marshal.FreeHGlobal(_pythonHome);
8485
_pythonHome = UcsMarshaler.Py3UnicodePy2StringtoPtr(value);
8586
Runtime.Py_SetPythonHome(_pythonHome);

‎src/runtime/runtime.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Diagnostics.Contracts;
23
using System.Runtime.InteropServices;
34
using System.Security;
45
using System.Text;
@@ -8,7 +9,6 @@
89

910
namespace Python.Runtime
1011
{
11-
1212
/// <summary>
1313
/// Encapsulates the low-level Python C API. Note that it is
1414
/// the responsibility of the caller to have acquired the GIL
@@ -106,7 +106,7 @@ public class Runtime
106106
internal static object IsFinalizingLock = new object();
107107
internal static bool IsFinalizing;
108108

109-
internal static bool Is32Bit = IntPtr.Size == 4;
109+
internal static bool Is32Bit => IntPtr.Size == 4;
110110

111111
// .NET core: System.Runtime.InteropServices.RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
112112
internal static bool IsWindows = Environment.OSVersion.Platform == PlatformID.Win32NT;
@@ -659,6 +659,7 @@ internal static unsafe void XDecref(IntPtr op)
659659
#endif
660660
}
661661

662+
[Pure]
662663
internal static unsafe long Refcount(IntPtr op)
663664
{
664665
var p = (void*)op;

0 commit comments

Comments
 (0)