Skip to content

Commit 6c82c7e

Browse files
author
dse
committed
Valid garbage collection implementation.
1 parent 07ff4ec commit 6c82c7e

File tree

10 files changed

+390
-53
lines changed

10 files changed

+390
-53
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ This document follows the conventions laid out in [Keep a CHANGELOG][].
2323
### Changed
2424

2525
### Fixed
26-
26+
- Fixed memory leaks, caused by non-working PyObject, PythonException, DelecateManager->Dispatcher finalizers.
2727
- Fixed secondary PythonEngine.Initialize call, all sensitive static variables now reseted.
2828
This is a hidden bug. Once python cleaning up enough memory, objects from previous engine run becomes corrupted.
2929
- Fixed Visual Studio 2017 compat (#434) for setup.py

‎src/embed_tests/Python.EmbeddingTest.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@
106106
<Compile Include="TestPyWith.cs" />
107107
<Compile Include="TestRuntime.cs" />
108108
<Compile Include="TestPyScope.cs" />
109+
<Compile Include="TestFinalizer.cs" />
109110
<Compile Include="TestsSuite.cs" />
110111
</ItemGroup>
111112
<ItemGroup>

‎src/embed_tests/TestFinalizer.cs‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
using System;
2+
using System.IO;
3+
using System.Threading;
4+
using System.Threading.Tasks;
5+
using NUnit.Framework;
6+
using Python.Runtime;
7+
8+
namespace Python.EmbeddingTest
9+
{
10+
public class TestFinalizer
11+
{
12+
[OneTimeSetUp]
13+
public void SetUp()
14+
{
15+
PythonEngine.Initialize();
16+
}
17+
18+
[OneTimeTearDown]
19+
public void Dispose()
20+
{
21+
PythonEngine.Shutdown();
22+
}
23+
24+
[Test]
25+
public void TestClrObjectFullRelease()
26+
{
27+
var gs = PythonEngine.BeginAllowThreads();
28+
29+
WeakReference weakRef;
30+
try
31+
{
32+
var weakRefCreateTask = Task.Factory.StartNew(() =>
33+
{
34+
using (Py.GIL())
35+
{
36+
byte[] testObject = new byte[100];
37+
var testObjectWeakReference = new WeakReference(testObject);
38+
39+
dynamic pyList = new PyList();
40+
pyList.append(testObject);
41+
return testObjectWeakReference;
42+
}
43+
});
44+
45+
weakRef = weakRefCreateTask.Result;
46+
}
47+
finally
48+
{
49+
PythonEngine.EndAllowThreads(gs);
50+
}
51+
52+
// Triggering C# finalizer (PyList ref should be scheduled to decref).
53+
GC.Collect();
54+
GC.WaitForPendingFinalizers();
55+
56+
// Forcing dec reference thread to wakeup and decref PyList.
57+
PythonEngine.EndAllowThreads(PythonEngine.BeginAllowThreads());
58+
Thread.Sleep(200);
59+
PythonEngine.CurrentRefDecrementer.WaitForPendingDecReferences();
60+
61+
// Now python free up GCHandle on CLRObject and subsequent GC should fully remove testObject.
62+
GC.Collect();
63+
GC.WaitForPendingFinalizers();
64+
65+
Assert.IsFalse(weakRef.IsAlive, "Clr object should be collected.");
66+
}
67+
}
68+
}

‎src/runtime/Python.Runtime.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@
129129
<Compile Include="pylong.cs" />
130130
<Compile Include="pynumber.cs" />
131131
<Compile Include="pyobject.cs" />
132+
<Compile Include="pyreferencedecrementer.cs" />
132133
<Compile Include="pyscope.cs" />
133134
<Compile Include="pysequence.cs" />
134135
<Compile Include="pystring.cs" />

‎src/runtime/delegatemanager.cs‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -185,28 +185,20 @@ public class Dispatcher
185185
{
186186
public IntPtr target;
187187
public Type dtype;
188+
private readonly PyReferenceDecrementer _referenceDecrementer;
188189

189190
public Dispatcher(IntPtr target, Type dtype)
190191
{
192+
_referenceDecrementer = PythonEngine.CurrentRefDecrementer;
193+
191194
Runtime.XIncref(target);
192195
this.target = target;
193196
this.dtype = dtype;
194197
}
195198

196199
~Dispatcher()
197200
{
198-
// We needs to disable Finalizers until it's valid implementation.
199-
// Current implementation can produce low probability floating bugs.
200-
return;
201-
202-
// Note: the managed GC thread can run and try to free one of
203-
// these *after* the Python runtime has been finalized!
204-
if (Runtime.Py_IsInitialized() > 0)
205-
{
206-
IntPtr gs = PythonEngine.AcquireLock();
207-
Runtime.XDecref(target);
208-
PythonEngine.ReleaseLock(gs);
209-
}
201+
_referenceDecrementer?.ScheduleDecRef(target);
210202
}
211203

212204
public object Dispatch(ArrayList args)

‎src/runtime/pyobject.cs‎

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ public class PyObject : DynamicObject, IDisposable
1616
{
1717
protected internal IntPtr obj = IntPtr.Zero;
1818
private bool disposed = false;
19+
private readonly PyReferenceDecrementer _referenceDecrementer;
1920

2021
/// <summary>
2122
/// PyObject Constructor
@@ -26,7 +27,7 @@ public class PyObject : DynamicObject, IDisposable
2627
/// and the reference will be DECREFed when the PyObject is garbage
2728
/// collected or explicitly disposed.
2829
/// </remarks>
29-
public PyObject(IntPtr ptr)
30+
public PyObject(IntPtr ptr): this()
3031
{
3132
obj = ptr;
3233
}
@@ -36,18 +37,15 @@ public PyObject(IntPtr ptr)
3637

3738
protected PyObject()
3839
{
40+
_referenceDecrementer = PythonEngine.CurrentRefDecrementer;
3941
}
4042

4143
// Ensure that encapsulated Python object is decref'ed appropriately
4244
// when the managed wrapper is garbage-collected.
4345

4446
~PyObject()
4547
{
46-
// We needs to disable Finalizers until it's valid implementation.
47-
// Current implementation can produce low probability floating bugs.
48-
return;
49-
50-
Dispose();
48+
Dispose(false);
5149
}
5250

5351

@@ -138,21 +136,43 @@ protected virtual void Dispose(bool disposing)
138136
{
139137
if (!disposed)
140138
{
141-
if (Runtime.Py_IsInitialized() > 0 && !Runtime.IsFinalizing)
139+
disposed = true;
140+
141+
if (disposing)
142142
{
143-
IntPtr gs = PythonEngine.AcquireLock();
144-
Runtime.XDecref(obj);
143+
try
144+
{
145+
if (Runtime.Py_IsInitialized() > 0 && !Runtime.IsFinalizing)
146+
{
147+
IntPtr gs = PythonEngine.AcquireLock();
148+
try
149+
{
150+
Runtime.XDecref(obj);
151+
obj = IntPtr.Zero;
152+
}
153+
finally
154+
{
155+
PythonEngine.ReleaseLock(gs);
156+
}
157+
}
158+
}
159+
catch
160+
{
161+
// Do nothing.
162+
}
163+
}
164+
else
165+
{
166+
_referenceDecrementer?.ScheduleDecRef(obj);
145167
obj = IntPtr.Zero;
146-
PythonEngine.ReleaseLock(gs);
147168
}
148-
disposed = true;
149169
}
150170
}
151171

152172
public void Dispose()
153173
{
154-
Dispose(true);
155174
GC.SuppressFinalize(this);
175+
Dispose(true);
156176
}
157177

158178

‎src/runtime/pyreferencedecrementer.cs‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
using System;
2+
using System.Collections.Concurrent;
3+
using System.Collections.Generic;
4+
using System.Diagnostics;
5+
using System.Threading;
6+
using System.Threading.Tasks;
7+
8+
namespace Python.Runtime
9+
{
10+
internal class PyReferenceDecrementer : IDisposable
11+
{
12+
private static readonly DedicatedThreadTaskScheduler DedicatedThreadTaskScheduler = new DedicatedThreadTaskScheduler();
13+
14+
private readonly BlockingCollection<IntPtr> _asyncDecRefQueue = new BlockingCollection<IntPtr>();
15+
16+
private CancellationTokenSource _cts;
17+
private CancellationToken _ct;
18+
private Task _backgroundWorkerTask;
19+
20+
public PyReferenceDecrementer()
21+
{
22+
InitDecRefThread();
23+
}
24+
25+
public void ScheduleDecRef(IntPtr pyRef)
26+
{
27+
// ReSharper disable once MethodSupportsCancellation
28+
_asyncDecRefQueue.Add(pyRef);
29+
}
30+
31+
internal void WaitForPendingDecReferences()
32+
{
33+
ShutdownDecRefThread();
34+
InitDecRefThread();
35+
}
36+
37+
private void ShutdownDecRefThread()
38+
{
39+
_cts?.Cancel();
40+
try
41+
{
42+
IntPtr ts = IntPtr.Zero;
43+
if (Runtime.PyGILState_GetThisThreadState() != IntPtr.Zero)
44+
{
45+
ts = Runtime.PyEval_SaveThread();
46+
}
47+
try
48+
{
49+
// ReSharper disable once MethodSupportsCancellation
50+
_backgroundWorkerTask.Wait();
51+
}
52+
catch (AggregateException ex)
53+
{
54+
if (!(ex.InnerException is OperationCanceledException))
55+
{
56+
throw;
57+
}
58+
}
59+
finally
60+
{
61+
if (ts != IntPtr.Zero)
62+
{
63+
Runtime.PyEval_RestoreThread(ts);
64+
}
65+
}
66+
}
67+
catch
68+
{
69+
// Just stopping background thread.
70+
}
71+
72+
_cts = null;
73+
_ct = default(CancellationToken);
74+
75+
_backgroundWorkerTask = null;
76+
}
77+
78+
private void InitDecRefThread()
79+
{
80+
_cts = new CancellationTokenSource();
81+
_ct = _cts.Token;
82+
83+
_backgroundWorkerTask = Task.Factory.StartNew(WorkerThread, _ct, TaskCreationOptions.LongRunning,
84+
DedicatedThreadTaskScheduler);
85+
}
86+
87+
private void WorkerThread()
88+
{
89+
while (true)
90+
{
91+
IntPtr refToDecrease = _asyncDecRefQueue.Take(_ct); ;
92+
93+
try
94+
{
95+
96+
IntPtr gs = PythonEngine.AcquireLock();
97+
try
98+
{
99+
do
100+
{
101+
Runtime.XDecref(refToDecrease);
102+
} while (_asyncDecRefQueue.TryTake(out refToDecrease));
103+
}
104+
finally
105+
{
106+
PythonEngine.ReleaseLock(gs);
107+
}
108+
}
109+
catch
110+
{
111+
// Nothing to do in this case.
112+
}
113+
}
114+
}
115+
116+
public void Dispose()
117+
{
118+
ShutdownDecRefThread();
119+
}
120+
}
121+
122+
123+
/// <summary>
124+
/// Scheduler that uses only one thread for all scheduled task.
125+
/// </summary>
126+
internal class DedicatedThreadTaskScheduler : TaskScheduler
127+
{
128+
private readonly BlockingCollection<Task> _queuedTasks = new BlockingCollection<Task>();
129+
130+
131+
/// <summary>
132+
/// Initializes a new instance of the <see cref="DedicatedThreadTaskScheduler"/> class.
133+
/// </summary>
134+
public DedicatedThreadTaskScheduler()
135+
{
136+
var thread = new Thread(WorkerThreadProc);
137+
thread.IsBackground = true;
138+
thread.Start();
139+
}
140+
141+
/// <inheritdoc/>
142+
protected override IEnumerable<Task> GetScheduledTasks()
143+
{
144+
return _queuedTasks;
145+
}
146+
147+
/// <inheritdoc/>
148+
protected override void QueueTask(Task task)
149+
{
150+
_queuedTasks.Add(task);
151+
}
152+
153+
/// <inheritdoc/>
154+
protected override bool TryExecuteTaskInline(Task task, bool taskWasPreviouslyQueued)
155+
{
156+
return false;
157+
}
158+
159+
private void WorkerThreadProc()
160+
{
161+
for (;;)
162+
{
163+
Task dequeuedTask = _queuedTasks.Take();
164+
165+
// This is synchronous execution.
166+
bool taskExecuted = TryExecuteTask(dequeuedTask);
167+
Debug.Assert(taskExecuted, "DedicatedThread task have some problem.");
168+
}
169+
}
170+
}
171+
}

0 commit comments

Comments
 (0)