| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -27,6 +27,7 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. | |||
| 27 | 27 | - 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]) | |
| 28 | 28 | - Added support for kwarg parameters when calling .NET methods from Python | |
| 29 | 29 | - 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. | ||
| 30 | 31 | ||
| 31 | 32 | ### Fixed | |
| 32 | 33 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -77,7 +77,7 @@ public void CollectBasicObject() | |||
| 77 | 77 | } | |
| 78 | 78 | try | |
| 79 | 79 | { | |
| 80 | - Finalizer.Instance.Collect(forceDispose: false); | ||
| 80 | + Finalizer.Instance.Collect(); | ||
| 81 | 81 | } | |
| 82 | 82 | finally | |
| 83 | 83 | { | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -35,13 +35,13 @@ internal CLRObject(object ob, IntPtr tp) | |||
| 35 | 35 | } | |
| 36 | 36 | ||
| 37 | 37 | ||
| 38 | - internal static CLRObject GetInstance(object ob, IntPtr pyType) | ||
| 38 | + static CLRObject GetInstance(object ob, IntPtr pyType) | ||
| 39 | 39 | { | |
| 40 | 40 | return new CLRObject(ob, pyType); | |
| 41 | 41 | } | |
| 42 | 42 | ||
| 43 | 43 | ||
| 44 | - internal static CLRObject GetInstance(object ob) | ||
| 44 | + static CLRObject GetInstance(object ob) | ||
| 45 | 45 | { | |
| 46 | 46 | ClassBase cc = ClassManager.GetClass(ob.GetType()); | |
| 47 | 47 | return GetInstance(ob, cc.tpHandle); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -28,9 +28,7 @@ public class ErrorArgs : EventArgs | |||
| 28 | 28 | public bool Enable { get; set; } | |
| 29 | 29 | ||
| 30 | 30 | 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; | ||
| 34 | 32 | ||
| 35 | 33 | #region FINALIZER_CHECK | |
| 36 | 34 | ||
@@ -75,19 +73,16 @@ private Finalizer() | |||
| 75 | 73 | Threshold = 200; | |
| 76 | 74 | } | |
| 77 | 75 | ||
| 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() | ||
| 79 | 81 | { | |
| 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(); | ||
| 91 | 86 | } | |
| 92 | 87 | ||
| 93 | 88 | public List<WeakReference> GetCollectedObjects() | |
@@ -101,62 +96,18 @@ internal void AddFinalizedObject(IPyDisposable obj) | |||
| 101 | 96 | { | |
| 102 | 97 | return; | |
| 103 | 98 | } | |
| 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 | + | ||
| 110 | 100 | #if FINALIZER_CHECK | |
| 111 | 101 | lock (_queueLock) | |
| 112 | 102 | #endif | |
| 113 | 103 | { | |
| 114 | - _objQueue.Enqueue(obj); | ||
| 115 | - } | ||
| 116 | - GC.ReRegisterForFinalize(obj); | ||
| 117 | - if (!_pending && _objQueue.Count >= Threshold) | ||
| 118 | - { | ||
| 119 | - AddPendingCollect(); | ||
| 104 | + this._objQueue.Enqueue(obj); | ||
| 120 | 105 | } | |
| 121 | 106 | } | |
| 122 | 107 | ||
| 123 | 108 | internal static void Shutdown() | |
| 124 | 109 | { | |
| 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(); | ||
| 160 | 111 | } | |
| 161 | 112 | ||
| 162 | 113 | private void DisposeAll() | |
@@ -178,12 +129,18 @@ private void DisposeAll() | |||
| 178 | 129 | try | |
| 179 | 130 | { | |
| 180 | 131 | obj.Dispose(); | |
| 181 | - Runtime.CheckExceptionOccurred(); | ||
| 182 | 132 | } | |
| 183 | 133 | catch (Exception e) | |
| 184 | 134 | { | |
| 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() | ||
| 187 | 144 | { | |
| 188 | 145 | Error = e | |
| 189 | 146 | }); | |
@@ -267,4 +224,15 @@ private void ValidateRefCount() | |||
| 267 | 224 | } | |
| 268 | 225 | #endif | |
| 269 | 226 | } | |
| 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 | + } | ||
| 270 | 238 | } | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -30,8 +30,6 @@ public class PyObject : DynamicObject, IEnumerable, IPyDisposable | |||
| 30 | 30 | #endif | |
| 31 | 31 | ||
| 32 | 32 | protected internal IntPtr obj = IntPtr.Zero; | |
| 33 | - private bool disposed = false; | ||
| 34 | - private bool _finalized = false; | ||
| 35 | 33 | ||
| 36 | 34 | internal BorrowedReference Reference => new BorrowedReference(obj); | |
| 37 | 35 | ||
@@ -49,6 +47,7 @@ public PyObject(IntPtr ptr) | |||
| 49 | 47 | if (ptr == IntPtr.Zero) throw new ArgumentNullException(nameof(ptr)); | |
| 50 | 48 | ||
| 51 | 49 | obj = ptr; | |
| 50 | + Finalizer.Instance.ThrottledCollect(); | ||
| 52 | 51 | #if TRACE_ALLOC | |
| 53 | 52 | Traceback = new StackTrace(1); | |
| 54 | 53 | #endif | |
@@ -64,6 +63,7 @@ internal PyObject(BorrowedReference reference) | |||
| 64 | 63 | if (reference.IsNull) throw new ArgumentNullException(nameof(reference)); | |
| 65 | 64 | ||
| 66 | 65 | obj = Runtime.SelfIncRef(reference.DangerousGetAddress()); | |
| 66 | + Finalizer.Instance.ThrottledCollect(); | ||
| 67 | 67 | #if TRACE_ALLOC | |
| 68 | 68 | Traceback = new StackTrace(1); | |
| 69 | 69 | #endif | |
@@ -74,6 +74,7 @@ internal PyObject(BorrowedReference reference) | |||
| 74 | 74 | [Obsolete("Please, always use PyObject(*Reference)")] | |
| 75 | 75 | protected PyObject() | |
| 76 | 76 | { | |
| 77 | + Finalizer.Instance.ThrottledCollect(); | ||
| 77 | 78 | #if TRACE_ALLOC | |
| 78 | 79 | Traceback = new StackTrace(1); | |
| 79 | 80 | #endif | |
@@ -87,12 +88,6 @@ protected PyObject() | |||
| 87 | 88 | { | |
| 88 | 89 | return; | |
| 89 | 90 | } | |
| 90 | - if (_finalized || disposed) | ||
| 91 | - { | ||
| 92 | - return; | ||
| 93 | - } | ||
| 94 | - // Prevent a infinity loop by calling GC.WaitForPendingFinalizers | ||
| 95 | - _finalized = true; | ||
| 96 | 91 | Finalizer.Instance.AddFinalizedObject(this); | |
| 97 | 92 | } | |
| 98 | 93 | ||
@@ -183,17 +178,41 @@ public T As<T>() | |||
| 183 | 178 | /// </remarks> | |
| 184 | 179 | protected virtual void Dispose(bool disposing) | |
| 185 | 180 | { | |
| 186 | - if (!disposed) | ||
| 181 | + if (this.obj == IntPtr.Zero) | ||
| 187 | 182 | { | |
| 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 | ||
| 189 | 211 | { | |
| 190 | - IntPtr gs = PythonEngine.AcquireLock(); | ||
| 191 | - Runtime.XDecref(obj); | ||
| 192 | - obj = IntPtr.Zero; | ||
| 193 | - PythonEngine.ReleaseLock(gs); | ||
| 212 | + Runtime.XDecref(this.obj); | ||
| 194 | 213 | } | |
| 195 | - disposed = true; | ||
| 196 | 214 | } | |
| 215 | + this.obj = IntPtr.Zero; | ||
| 197 | 216 | } | |
| 198 | 217 | ||
| 199 | 218 | public void Dispose() | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -80,6 +80,7 @@ public static string PythonHome | |||
| 80 | 80 | } | |
| 81 | 81 | set | |
| 82 | 82 | { | |
| 83 | + // this value is null in the beginning | ||
| 83 | 84 | Marshal.FreeHGlobal(_pythonHome); | |
| 84 | 85 | _pythonHome = UcsMarshaler.Py3UnicodePy2StringtoPtr(value); | |
| 85 | 86 | Runtime.Py_SetPythonHome(_pythonHome); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1,4 +1,5 @@ | |||
| 1 | 1 | using System; | |
| 2 | + using System.Diagnostics.Contracts; | ||
| 2 | 3 | using System.Runtime.InteropServices; | |
| 3 | 4 | using System.Security; | |
| 4 | 5 | using System.Text; | |
@@ -8,7 +9,6 @@ | |||
| 8 | 9 | ||
| 9 | 10 | namespace Python.Runtime | |
| 10 | 11 | { | |
| 11 | - | ||
| 12 | 12 | /// <summary> | |
| 13 | 13 | /// Encapsulates the low-level Python C API. Note that it is | |
| 14 | 14 | /// the responsibility of the caller to have acquired the GIL | |
@@ -106,7 +106,7 @@ public class Runtime | |||
| 106 | 106 | internal static object IsFinalizingLock = new object(); | |
| 107 | 107 | internal static bool IsFinalizing; | |
| 108 | 108 | ||
| 109 | - internal static bool Is32Bit = IntPtr.Size == 4; | ||
| 109 | + internal static bool Is32Bit => IntPtr.Size == 4; | ||
| 110 | 110 | ||
| 111 | 111 | // .NET core: System.Runtime.InteropServices.RuntimeInformation.IsOSPlatform(OSPlatform.Windows) | |
| 112 | 112 | internal static bool IsWindows = Environment.OSVersion.Platform == PlatformID.Win32NT; | |
@@ -659,6 +659,7 @@ internal static unsafe void XDecref(IntPtr op) | |||
| 659 | 659 | #endif | |
| 660 | 660 | } | |
| 661 | 661 | ||
| 662 | + [Pure] | ||
| 662 | 663 | internal static unsafe long Refcount(IntPtr op) | |
| 663 | 664 | { | |
| 664 | 665 | var p = (void*)op; | |
| Back | FazBrowse Home | New Git URL |
0 commit comments