Commit 885cd3e
committed
Fix memory leak in finalizer
- `finalizer` was leaking `_pendingArgs` (global memory) when the call
to `Py_AddPendingCall` was unsuccessful.
- Changing `lock` for `Monitor.TryEnter` at `AddPendingCollect()` so
threads don't block each other.1 parent f83c884 commit 885cd3e
1 file changed
Lines changed: 30 additions & 20 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -41,7 +41,7 @@ struct PendingArgs | |||
| 41 | 41 | private ConcurrentQueue<IPyDisposable> _objQueue = new ConcurrentQueue<IPyDisposable>(); | |
| 42 | 42 | private bool _pending = false; | |
| 43 | 43 | private readonly object _collectingLock = new object(); | |
| 44 | - private IntPtr _pendingArgs; | ||
| 44 | + private IntPtr _pendingArgs = IntPtr.Zero; | ||
| 45 | 45 | ||
| 46 | 46 | #region FINALIZER_CHECK | |
| 47 | 47 | ||
@@ -128,7 +128,7 @@ internal void AddFinalizedObject(IPyDisposable obj) | |||
| 128 | 128 | _objQueue.Enqueue(obj); | |
| 129 | 129 | } | |
| 130 | 130 | GC.ReRegisterForFinalize(obj); | |
| 131 | - if (_objQueue.Count >= Threshold) | ||
| 131 | + if (!_pending && _objQueue.Count >= Threshold) | ||
| 132 | 132 | { | |
| 133 | 133 | AddPendingCollect(); | |
| 134 | 134 | } | |
@@ -164,26 +164,28 @@ internal static void Shutdown() | |||
| 164 | 164 | ||
| 165 | 165 | private void AddPendingCollect() | |
| 166 | 166 | { | |
| 167 | - if (_pending) | ||
| 167 | + if(Monitor.TryEnter(_collectingLock)) | ||
| 168 | 168 | { | |
| 169 | - return; | ||
| 170 | - } | ||
| 171 | - lock (_collectingLock) | ||
| 172 | - { | ||
| 173 | - if (_pending) | ||
| 169 | + try | ||
| 174 | 170 | { | |
| 175 | - return; | ||
| 171 | + if (!_pending) | ||
| 172 | + { | ||
| 173 | + _pending = true; | ||
| 174 | + var args = new PendingArgs { cancelled = false }; | ||
| 175 | + _pendingArgs = Marshal.AllocHGlobal(Marshal.SizeOf(typeof(PendingArgs))); | ||
| 176 | + Marshal.StructureToPtr(args, _pendingArgs, false); | ||
| 177 | + IntPtr func = Marshal.GetFunctionPointerForDelegate(_collectAction); | ||
| 178 | + if (Runtime.Py_AddPendingCall(func, _pendingArgs) != 0) | ||
| 179 | + { | ||
| 180 | + // Full queue, append next time | ||
| 181 | + FreePendingArgs(); | ||
| 182 | + _pending = false; | ||
| 183 | + } | ||
| 184 | + } | ||
| 176 | 185 | } | |
| 177 | - _pending = true; | ||
| 178 | - var args = new PendingArgs() { cancelled = false }; | ||
| 179 | - IntPtr p = Marshal.AllocHGlobal(Marshal.SizeOf(typeof(PendingArgs))); | ||
| 180 | - Marshal.StructureToPtr(args, p, false); | ||
| 181 | - _pendingArgs = p; | ||
| 182 | - IntPtr func = Marshal.GetFunctionPointerForDelegate(_collectAction); | ||
| 183 | - if (Runtime.Py_AddPendingCall(func, p) != 0) | ||
| 186 | + finally | ||
| 184 | 187 | { | |
| 185 | - // Full queue, append next time | ||
| 186 | - _pending = false; | ||
| 188 | + Monitor.Exit(_collectingLock); | ||
| 187 | 189 | } | |
| 188 | 190 | } | |
| 189 | 191 | } | |
@@ -205,8 +207,8 @@ private static int OnPendingCollect(IntPtr arg) | |||
| 205 | 207 | } | |
| 206 | 208 | finally | |
| 207 | 209 | { | |
| 210 | + Instance.FreePendingArgs(); | ||
| 208 | 211 | Instance.ResetPending(); | |
| 209 | - Marshal.FreeHGlobal(arg); | ||
| 210 | 212 | } | |
| 211 | 213 | return 0; | |
| 212 | 214 | } | |
@@ -244,12 +246,20 @@ private void DisposeAll() | |||
| 244 | 246 | } | |
| 245 | 247 | } | |
| 246 | 248 | ||
| 249 | + private void FreePendingArgs() | ||
| 250 | + { | ||
| 251 | + if (_pendingArgs != IntPtr.Zero) | ||
| 252 | + { | ||
| 253 | + Marshal.FreeHGlobal(_pendingArgs); | ||
| 254 | + _pendingArgs = IntPtr.Zero; | ||
| 255 | + } | ||
| 256 | + } | ||
| 257 | + | ||
| 247 | 258 | private void ResetPending() | |
| 248 | 259 | { | |
| 249 | 260 | lock (_collectingLock) | |
| 250 | 261 | { | |
| 251 | 262 | _pending = false; | |
| 252 | - _pendingArgs = IntPtr.Zero; | ||
| 253 | 263 | } | |
| 254 | 264 | } | |
| 255 | 265 | ||
0 commit comments