Skip to content

Commit fa4135a

Browse files
Don't hold a lock while lazy Python members run Python code
PyDecimalType and the converter's datetime/tzinfo constructors were Lazy<T> in ExecutionAndPublication mode. Their factories run Python code with the GIL held, and that code can hand the GIL to another thread. A thread that then reached the same Lazy blocked on its lock while holding the GIL, so the factory could never finish: a deadlock. PublicationOnly lets that thread run the factory itself; the first published value wins. Closes #153 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent ba1b2ea commit fa4135a

4 files changed

Lines changed: 105 additions & 3 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ Instead, `PyIterable` does that.
9494

9595
### Fixed
9696

97+
- Fix deadlock when several threads holding the GIL first resolve the Python `Decimal`, `datetime` or `tzinfo` types at the same time
9798
- Fix incorrect dereference of wrapper object in `tp_repr`, which may result in a program crash
9899
- Fixed parameterless .NET constructor being silently called when a matching constructor overload is not found ([#238][i238])
99100
- Fix incorrect dereference in params array handling
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
using System;
2+
using System.Threading;
3+
using NUnit.Framework;
4+
using Python.Runtime;
5+
6+
namespace Python.EmbeddingTest
7+
{
8+
public class TestLazyPyMembers
9+
{
10+
public static class OverloadedMethods
11+
{
12+
public static string Call(string value) => "string";
13+
public static string Call(object value) => "object";
14+
}
15+
16+
[OneTimeSetUp]
17+
public void SetUp()
18+
{
19+
PythonEngine.Initialize();
20+
}
21+
22+
[OneTimeTearDown]
23+
public void Dispose()
24+
{
25+
PythonEngine.Shutdown();
26+
}
27+
28+
/// <summary>
29+
/// Resolving PyDecimalType imports _pydecimal, which runs Python code that can hand the GIL
30+
/// to another thread. A thread that reaches PyDecimalType meanwhile must not block on it
31+
/// while holding the GIL, or the importing thread can never finish.
32+
/// </summary>
33+
[Test]
34+
public void ConcurrentFirstResolutionOfDecimalTypeDoesNotDeadlock()
35+
{
36+
using var scope = Py.CreateScope();
37+
scope.Exec(@"
38+
import sys
39+
import threading
40+
from Python.EmbeddingTest import TestLazyPyMembers
41+
OverloadedMethods = TestLazyPyMembers.OverloadedMethods
42+
43+
importing = threading.Event()
44+
45+
class SlowDecimalImport:
46+
def find_spec(self, name, path, target=None):
47+
if name == '_pydecimal':
48+
importing.set()
49+
# keep the import open, with the GIL released, until the other thread reaches PyDecimalType
50+
threading.Event().wait(1)
51+
return None
52+
53+
finder = SlowDecimalImport()
54+
sys.modules.pop('_pydecimal', None)
55+
sys.meta_path.insert(0, finder)
56+
");
57+
Assert.IsFalse(Runtime.Runtime.PyDecimalType.IsValueCreated);
58+
59+
var results = new string[2];
60+
var errors = new Exception[2];
61+
Thread Start(int index, string code) => new Thread(() =>
62+
{
63+
try
64+
{
65+
using (Py.GIL())
66+
{
67+
results[index] = scope.Eval<string>(code);
68+
}
69+
}
70+
catch (Exception e)
71+
{
72+
errors[index] = e;
73+
}
74+
});
75+
var first = Start(0, "OverloadedMethods.Call([])");
76+
var second = Start(1, "importing.wait(10) and OverloadedMethods.Call([])");
77+
78+
// the fixture initializes the engine with the GIL held on this thread:
79+
// release it so the worker threads can acquire it
80+
var ts = PythonEngine.BeginAllowThreads();
81+
try
82+
{
83+
second.Start();
84+
first.Start();
85+
Assert.IsTrue(first.Join(TimeSpan.FromSeconds(30)), "first thread deadlocked");
86+
Assert.IsTrue(second.Join(TimeSpan.FromSeconds(30)), "second thread deadlocked");
87+
}
88+
finally
89+
{
90+
PythonEngine.EndAllowThreads(ts);
91+
}
92+
scope.Exec("sys.meta_path.remove(finder)");
93+
94+
Assert.IsNull(errors[0]);
95+
Assert.IsNull(errors[1]);
96+
Assert.AreEqual("object", results[0]);
97+
Assert.AreEqual("object", results[1]);
98+
}
99+
}
100+
}

‎src/runtime/Converter.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Runtime.InteropServices;
99
using System.Security;
1010
using System.Text;
11+
using System.Threading;
1112

1213
using Python.Runtime.Native;
1314
using System.Linq;
@@ -149,7 +150,7 @@ def __reduce_ex__(self, protocol):
149150
var result = Runtime.PyObject_GetAttrString(dateTimeSubclassMod, "datetime").MoveToPyObject();
150151
PythonException.ThrowIfIsNull(result);
151152
return result;
152-
});
153+
}, LazyThreadSafetyMode.PublicationOnly);
153154

154155
timeSpanCtor = Runtime.PyObject_GetAttrString(dateTimeMod.Borrow(), "timedelta").MoveToPyObject();
155156
PythonException.ThrowIfIsNull(timeSpanCtor);
@@ -172,7 +173,7 @@ def dst (self, dt):
172173
var result = Runtime.PyObject_GetAttrString(tzInfoMod, "GMT").MoveToPyObject();
173174
PythonException.ThrowIfIsNull(result);
174175
return result;
175-
});
176+
}, LazyThreadSafetyMode.PublicationOnly);
176177

177178
pyTupleNoKind = Runtime.PyTuple_New(7).MoveToPyObject();
178179
pyTupleKind = Runtime.PyTuple_New(8).MoveToPyObject();

‎src/runtime/Runtime.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ private static void InitPyMembers()
244244
var op = PyObject_CallObject(decimalCtor.BorrowNullable(), BorrowedReference.Null).MoveToPyObject();
245245
SetPyMemberTypeOf(out var result, op);
246246
return result;
247-
});
247+
}, LazyThreadSafetyMode.PublicationOnly);
248248

249249
_PyObject_NextNotImplemented = Get_PyObject_NextNotImplemented();
250250
{

0 commit comments

Comments
 (0)