8000 Merge pull request #29 from QuantConnect/fix-memory-leak-finalizer-v2 · saaib/pythonnet@385a5ed · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit 385a5ed

Browse files
jaredbroadjaredbroad
authored
Merge pull request QuantConnect#29 from QuantConnect/fix-memory-leak-finalizer-v2
Fix memory leak finalizer
2 parents 928db02 + b1e8444 commit 385a5ed

File tree

8 files changed

+54
-120
lines changed

8 files changed

+54
-120
lines changed

‎.bumpversion.cfg

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
[bumpversion]
2-
current_version = 1.0.5.19
2+
current_version = 1.0.5.20
33
parse = (?P<major>\d+)\.(?P<minor>\d+)\.(?P<patch>\d+)(\.(?P<release>[a-z]+)(?P<dev>\d+))?
44
serialize =
55
{major}.{minor}.{patch}.{release}{dev}

‎conda.recipe/meta.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package:
22
name: pythonnet
3-
version: "1.0.5.19"
3+
version: "1.0.5.20"
44

55
build:
66
skip: True # [not win]

‎setup.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,7 @@ def run(self):
485485

486486
setup(
487487
name="pythonnet",
488-
version="1.0.5.19",
488+
version="1.0.5.20",
489489
description=".Net and Mono integration for Python",
490490
url='https://pythonnet.github.io/',
491491
license='MIT',

‎src/SharedAssemblyInfo.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,4 @@
2525
// Version Information. Keeping it simple. May need to revisit for Nuget
2626
// See: https://codingforsmarties.wordpress.com/2016/01/21/how-to-version-assemblies-destined-for-nuget/
2727
// AssemblyVersion can only be numeric
28-
[assembly: AssemblyVersion("1.0.5.19")]
28+
[assembly: AssemblyVersion("1.0.5.20")]

‎src/clrmodule/ClrModule.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public static void initclr()
5353
{
5454
#if USE_PYTHON_RUNTIME_VERSION
5555
// Has no effect until SNK works. Keep updated anyways.
56-
Version = new Version("1.0.5.19"),
56+
Version = new Version("1.0.5.20"),
5757
#endif
5858
CultureInfo = CultureInfo.InvariantCulture
5959
};

‎src/embed_tests/TestFinalizer.cs

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,18 @@ public void CollectBasicObject()
3636
{
3737
Assert.IsTrue(Finalizer.Instance.Enable);
3838

39-
int thId = Thread.CurrentThread.ManagedThreadId;
4039
Finalizer.Instance.Threshold = 1;
4140
bool called = false;
41+
var objectCount = 0;
4242
EventHandler<Finalizer.CollectArgs> handler = (s, e) =>
4343
{
44-
Assert.AreEqual(thId, Thread.CurrentThread.ManagedThreadId);
45-
Assert.GreaterOrEqual(e.ObjectCount, 1);
44+
objectCount = e.ObjectCount;
4645
called = true;
4746
};
4847

48+
Assert.IsFalse(called);
49+
Finalizer.Instance.CollectOnce += handler;
50+
4951
WeakReference shortWeak;
5052
WeakReference longWeak;
5153
{
@@ -61,18 +63,16 @@ public void CollectBasicObject()
6163
Assert.NotZero(garbage.Count);
6264
Assert.IsTrue(garbage.Any(T => ReferenceEquals(T.Target, longWeak.Target)));
6365
}
64-
65-
Assert.IsFalse(called);
66-
Finalizer.Instance.CollectOnce += handler;
6766
try
6867
{
69-
Finalizer.Instance.CallPendingFinalizers();
68+
Finalizer.Instance.Collect(forceDispose: false);
7069
}
7170
finally
7271
{
7372
Finalizer.Instance.CollectOnce -= handler;
7473
}
7574
Assert.IsTrue(called);
75+
Assert.GreaterOrEqual(objectCount, 1);
7676
}
7777

7878
private static void MakeAGarbage(out WeakReference shortWeak, out WeakReference longWeak)
@@ -85,7 +85,7 @@ private static void MakeAGarbage(out WeakReference shortWeak, out WeakReference
8585

8686
private static long CompareWithFinalizerOn(PyObject pyCollect, bool enbale)
8787
{
88-
// Must larger than 512 bytes make sure Python use
88+
// Must larger than 512 bytes make sure Python use
8989
string str = new string('1', 1024);
9090
Finalizer.Instance.Enable = true;
9191
FullGCCollect();
@@ -164,10 +164,11 @@ internal static void CreateMyPyObject(IntPtr op)
164164
public void ErrorHandling()
165165
{
166166
bool called = false;
167+
var errorMessage = "";
167168
EventHandler<Finalizer.ErrorArgs> handleFunc = (sender, args) =>
168169
{
169170
called = true;
170-
Assert.AreEqual(args.Error.Message, "MyPyObject");
171+
errorMessage = args.Error.Message;
171172
};
172173
Finalizer.Instance.Threshold = 1;
173174
Finalizer.Instance.ErrorHandler += handleFunc;
@@ -193,6 +194,7 @@ public void ErrorHandling()
193194
{
194195
Finalizer.Instance.ErrorHandler -= handleFunc;
195196
}
197+
Assert.AreEqual(errorMessage, "MyPyObject");
196198
}
197199

198200
[Test]

‎src/runtime/finalizer.cs

Lines changed: 37 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
using System;
22
using System.Collections.Concurrent;
33
using System.Collections.Generic;
4-
using System.Diagnostics;
54
using System.Linq;
6-
using System.Runtime.InteropServices;
75
using System.Threading;
6+
using System.Threading.Tasks;
87

98
namespace Python.Runtime
109
{
@@ -28,20 +27,10 @@ public class ErrorArgs : EventArgs
2827
public int Threshold { get; set; }
2928
public bool Enable { get; set; }
3029

31-
[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)]
32-
struct PendingArgs
33-
{
34-
public bool cancelled;
35-
}
36-
37-
[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
38-
private delegate int PendingCall(IntPtr arg);
39-
private readonly PendingCall _collectAction;
40< 1C6A span class="diff-text-marker">-
4130
private ConcurrentQueue<IPyDisposable> _objQueue = new ConcurrentQueue<IPyDisposable>();
4231
private bool _pending = false;
4332
private readonly object _collectingLock = new object();
44-
private IntPtr _pendingArgs = IntPtr.Zero;
33+
private Task _finalizerTask;
4534

4635
#region FINALIZER_CHECK
4736

@@ -84,23 +73,23 @@ private Finalizer()
8473
{
8574
Enable = true;
8675
Threshold = 200;
87-
_collectAction = OnPendingCollect;
8876
}
8977

90-
public void CallPendingFinalizers()
78+
public void Collect(bool forceDispose = true)
9179
{
92-
if (Thread.CurrentThread.ManagedThreadId != Runtime.MainManagedThreadId)
80+
if (Instance._finalizerTask != null
81+
&& !Instance._finalizerTask.IsCompleted)
9382
{
94-
throw new Exception("PendingCall should execute in main Python thread");
83+
using (Py.GIL())
84+
{
85+
var ts = PythonEngine.BeginAllowThreads();
86+
Instance._finalizerTask.Wait();
87+
PythonEngine.EndAllowThreads(ts);
88+
}
9589
}
96-
Runtime.Py_MakePendingCalls();
97-
}
98-
99-
public void Collect()
100-
{
101-
using (var gilState = new Py.GILState())
90+
else if (forceDispose)
10291
{
103-
DisposeAll();
92+
Instance.DisposeAll();
10493
}
10594
}
10695

@@ -141,25 +130,7 @@ internal static void Shutdown()
141130
Instance._objQueue = new ConcurrentQueue<IPyDisposable>();
142131
return;
143132
}
144-
Instance.DisposeAll();
145-
if (Thread.CurrentThread.ManagedThreadId != Runtime.MainManagedThreadId)
146-
{
147-
if (Instance._pendingArgs == IntPtr.Zero)
148-
{
149-
Instance.ResetPending();
150-
return;
151-
}
152-
// Not in main thread just cancel the pending operation to avoid error in different domain
153-
// It will make a memory leak
154-
unsafe
155-
{
156-
PendingArgs* args = (PendingArgs*)Instance._pendingArgs;
157-
args->cancelled = true;
158-
}
159-
Instance.ResetPending();
160-
return;
161-
}
162-
Instance.CallPendingFinalizers();
133+
Instance.Collect(forceDispose: true);
163134
}
164135

165136
private void AddPendingCollect()
@@ -171,16 +142,14 @@ private void AddPendingCollect()
171142
if (!_pending)
172143
{
173144
_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)
145+
// should already be complete but just in case
146+
_finalizerTask?.Wait();
147+
148+
_finalizerTask = Task.Factory.StartNew(() =>
179149
{
180-
// Full queue, append next time
181-
FreePendingArgs();
150+
Instance.DisposeAll();
182151
_pending = false;
183-
}
152+
});
184153
}
185154
}
186155
finally
@@ -190,29 +159,6 @@ private void AddPendingCollect()
190159
}
191160
}
192161

193-
private static int OnPendingCollect(IntPtr arg)
194-
{
195-
Debug.Assert(arg == Instance._pendingArgs);
196-
try
197-
{
198-
unsafe
199-
{
200-
PendingArgs* pendingArgs = (PendingArgs*)arg;
201-
if (pendingArgs->cancelled)
202-
{
203-
return 0;
204-
}
205-
}
206-
Instance.DisposeAll();
207-
}
208-
finally
209-
{
210-
Instance.FreePendingArgs();
211-
Instance.ResetPending();
212-
}
213-
return 0;
214-
}
215-
216162
private void DisposeAll()
217163
{
218164
CollectOnce?.Invoke(this, new CollectArgs()
@@ -223,46 +169,32 @@ private void DisposeAll()
223169
lock (_queueLock)
224170
#endif
225171
{
172+
using (Py.GIL())
173+
{
226174
#if FINALIZER_CHECK
227-
ValidateRefCount();
175+
ValidateRefCount();
228176
#endif
229-
IPyDisposable obj;
230-
while (_objQueue.TryDequeue(out obj))
231-
{
232-
try
233-
{
234-
obj.Dispose();
235-
Runtime.CheckExceptionOccurred();
236-
}
237-
catch (Exception e)
177+
IPyDisposable obj;
178+
while (_objQueue.TryDequeue(out obj))
238179
{
239-
// We should not bother the main thread
240-
ErrorHandler?.Invoke(this, new ErrorArgs()
180+
try
241181
{
242-
Error = e
243-
});
182+
obj.Dispose();
183+
Runtime.CheckExceptionOccurred();
184+
}
185+
catch (Exception e)
186+
{
187+
// We should not bother the main thread
188+
ErrorHandler?.Invoke(this, new ErrorArgs()
189+
{
190+
Error = e
191+
});
192+
}
244193
}
245194
}
246195
}
247196
}
248197

249-
private void FreePendingArgs()
250-
{
251-
if (_pendingArgs != IntPtr.Zero)
252-
{
253-
Marshal.FreeHGlobal(_pendingArgs);
254-
_pendingArgs = IntPtr.Zero;
255-
}
256-
}
257-
258-
private void ResetPending()
259-
{
260-
lock (_collectingLock)
261-
{
262-
_pending = false;
263-
}
264-
}
265-
266198
#if FINALIZER_CHECK
267199
private void ValidateRefCount()
268200
{

‎src/runtime/resources/clr.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
Code in this module gets loaded into the main clr module.
33
"""
44

5-
__version__ = "1.0.5.19"
5+
__version__ = "1.0.5.20"
66

77

88
class clrproperty(object):

0 commit comments

Comments
 (0)
0