Skip to content

Commit bfd07e0

Browse files
braxtonmckeebraxtonmckee
committed
Fix a deadlock acquiring modules from C code.
We had static references to builtin modules being initialized in C code. These get guarded by an implicit lock on construction. If you call back into the interpreter whilst holding this lock you can deadlock. Instead we make a global memo guarded by its own lock and ensure that we never call the interprer under lock.
1 parent 470a5b3 commit bfd07e0

10 files changed

+125
-181
lines changed

‎typed_python/CompilerVisibleObjectVisitor.hpp‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -363,8 +363,7 @@ class CompilerVisibleObjectVisitor {
363363
static bool isPyObjectGloballyIdentifiable(PyObject* h) {
364364
PyEnsureGilAcquired getTheGil;
365365

366-
static PyObject* sysModule = PyImport_ImportModule("sys");
367-
static PyObject* sysModuleModules = PyObject_GetAttrString(sysModule, "modules");
366+
PyObject* sysModuleModules = staticPythonInstance("sys", "modules");
368367

369368
if (PyObject_HasAttrString(h, "__module__") && PyObject_HasAttrString(h, "__name__")) {
370369
PyObjectStealer moduleName(PyObject_GetAttrString(h, "__module__"));
@@ -410,8 +409,8 @@ class CompilerVisibleObjectVisitor {
410409
// in question should get deleted ever...
411410
PyEnsureGilAcquired getTheGil;
412411

413-
static PyObject* builtinsModule = ::builtinsModule();
414-
static PyObject* builtinsModuleDict = PyObject_GetAttrString(builtinsModule, "__dict__");
412+
PyObject* builtinsModule = staticPythonInstance("builtins", "");
413+
PyObject* builtinsModuleDict = staticPythonInstance("builtins", "__dict__");
415414

416415
// handle basic constants
417416
return (
@@ -560,8 +559,7 @@ class CompilerVisibleObjectVisitor {
560559
return;
561560
}
562561

563-
static PyObject* osModule = ::osModule();
564-
static PyObject* environType = PyObject_GetAttrString(osModule, "_Environ");
562+
PyObject* environType = staticPythonInstance("os", "_Environ");
565563

566564
if (obj.pyobj()->ob_type == (PyTypeObject*)environType) {
567565
// don't ever hash the environment.
@@ -583,8 +581,7 @@ class CompilerVisibleObjectVisitor {
583581

584582
// don't walk into canonical modules
585583
if (PyModule_Check(obj.pyobj())) {
586-
static PyObject* sysModule = ::sysModule();
587-
static PyObject* sysModuleModules = PyObject_GetAttrString(sysModule, "modules");
584+
PyObject* sysModuleModules = staticPythonInstance("sys", "modules");
588585

589586
PyObjectStealer name(PyObject_GetAttrString(obj.pyobj(), "__name__"));
590587
if (name) {
@@ -761,11 +758,9 @@ class CompilerVisibleObjectVisitor {
761758
return;
762759
}
763760

764-
static PyObject* weakrefModule = ::weakrefModule();
765-
static PyObject* weakSetType = PyObject_GetAttrString(weakrefModule, "WeakSet");
766-
static PyObject* weakKeyDictType = PyObject_GetAttrString(weakrefModule, "WeakKeyDictionary");
767-
static PyObject* weakValueDictType = PyObject_GetAttrString(weakrefModule, "WeakValueDictionary");
768-
761+
PyObject* weakSetType = staticPythonInstance("weakref", "WeakSet");
762+
PyObject* weakKeyDictType = staticPythonInstance("weakref", "WeakKeyDictionary");
763+
PyObject* weakValueDictType = staticPythonInstance("weakref", "WeakValueDictionary");
769764

770765
if (
771766
// dict, set and list are all mutable - we can't rely on their contents,

‎typed_python/MutuallyRecursiveTypeGroup.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -886,8 +886,8 @@ ShaHash MutuallyRecursiveTypeGroup::shaHashOfSimplePyObjectConstant(PyObject* h)
886886
return ShaHash(100, 6) + ShaHash::SHA1(c, s);
887887
}
888888

889-
static PyObject* builtinsModule = ::builtinsModule();
890-
static PyObject* builtinsModuleDict = PyObject_GetAttrString(builtinsModule, "__dict__");
889+
PyObject* builtinsModule = staticPythonInstance("builtins", "");
890+
PyObject* builtinsModuleDict = staticPythonInstance("builtins", "__dict__");
891891

892892
if (h == builtinsModule) {
893893
return ShaHash(100, 8);

‎typed_python/PyFunctionInstance.cpp‎

Lines changed: 10 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -436,27 +436,9 @@ std::pair<bool, PyObject*> PyFunctionInstance::dispatchFunctionCallToNative(
436436
}
437437
}
438438

439-
static PyObject* runtimeModule = ::runtimeModule();
440-
441-
if (!runtimeModule) {
442-
throw std::runtime_error("Internal error: couldn't find typed_python.compiler.runtime");
443-
}
444-
445-
PyObject* runtimeClass = PyObject_GetAttrString(runtimeModule, "Runtime");
446-
447-
if (!runtimeClass) {
448-
throw std::runtime_error("Internal error: couldn't find typed_python.compiler.runtime.Runtime");
449-
}
450-
451-
PyObject* singleton = PyObject_CallMethod(runtimeClass, "singleton", "");
452-
453-
if (!singleton) {
454-
if (PyErr_Occurred()) {
455-
PyErr_Clear();
456-
}
457-
458-
throw std::runtime_error("Internal error: couldn't call typed_python.compiler.runtime.Runtime.singleton");
459-
}
439+
PyObject* singleton = staticPythonInstance(
440+
"typed_python.compiler.runtime", "Runtime.singleton()"
441+
);
460442

461443
PyObjectStealer arguments(mapper.extractFunctionArgumentValues());
462444

@@ -575,29 +557,9 @@ std::pair<bool, PyObject*> PyFunctionInstance::dispatchFunctionCallToCompiledSpe
575557

576558
// static
577559
PyObject* PyFunctionInstance::createOverloadPyRepresentation(Function* f) {
578-
static PyObject* internalsModule = ::internalsModule();
579-
580-
if (!internalsModule) {
581-
throw std::runtime_error("Internal error: couldn't find typed_python.internals");
582-
}
583-
584-
static PyObject* funcOverload = PyObject_GetAttrString(internalsModule, "FunctionOverload");
585-
586-
if (!funcOverload) {
587-
throw std::runtime_error("Internal error: couldn't find typed_python.internals.FunctionOverload");
588-
}
589-
590-
static PyObject* closureVariableCellLookupSingleton = PyObject_GetAttrString(internalsModule, "CellAccess");
591-
592-
if (!closureVariableCellLookupSingleton) {
593-
throw std::runtime_error("Internal error: couldn't find typed_python.internals.CellAccess");
594-
}
595-
596-
static PyObject* funcOverloadArg = PyObject_GetAttrString(internalsModule, "FunctionOverloadArg");
597-
598-
if (!funcOverloadArg) {
599-
throw std::runtime_error("Internal error: couldn't find typed_python.internals.FunctionOverloadArg");
600-
}
560+
PyObject* funcOverload = staticPythonInstance("typed_python.internals", "FunctionOverload");
561+
PyObject* closureVariableCellLookupSingleton = staticPythonInstance("typed_python.internals", "CellAccess");
562+
PyObject* funcOverloadArg = staticPythonInstance("typed_python.internals", "FunctionOverloadArg");
601563

602564
PyObjectStealer overloadTuple(PyTuple_New(f->getOverloads().size()));
603565

@@ -1011,27 +973,9 @@ PyObject* PyFunctionInstance::overload(PyObject* funcObj, PyObject* args, PyObje
1011973

1012974
/* static */
1013975
PyObject* PyFunctionInstance::resultTypeFor(PyObject* funcObj, PyObject* args, PyObject* kwargs) {
1014-
static PyObject* runtimeModule = ::runtimeModule();
1015-
1016-
if (!runtimeModule) {
1017-
throw std::runtime_error("Internal error: couldn't find typed_python.compiler.runtime");
1018-
}
1019-
1020-
static PyObject* runtimeClass = PyObject_GetAttrString(runtimeModule, "Runtime");
1021-
1022-
if (!runtimeClass) {
1023-
throw std::runtime_error("Internal error: couldn't find typed_python.compiler.runtime.Runtime");
1024-
}
1025-
1026-
static PyObject* singleton = PyObject_CallMethod(runtimeClass, "singleton", "");
1027-
1028-
if (!singleton) {
1029-
if (PyErr_Occurred()) {
1030-
PyErr_Clear();
1031-
}
1032-
1033-
throw std::runtime_error("Internal error: couldn't call typed_python.compiler.runtime.Runtime.singleton");
1034-
}
976+
PyObject* singleton = staticPythonInstance(
977+
"typed_python.compiler.runtime", "Runtime.singleton()"
978+
);
1035979

1036980
if (!kwargs) {
1037981
static PyObject* emptyDict = PyDict_New();
@@ -1178,19 +1122,7 @@ Function* PyFunctionInstance::convertPythonObjectToFunctionType(
11781122
return memo_it->second;
11791123
}
11801124

1181-
static PyObject* internalsModule = ::internalsModule();
1182-
1183-
if (!internalsModule) {
1184-
PyErr_SetString(PyExc_TypeError, "Internal error: couldn't find typed_python.internals");
1185-
return nullptr;
1186-
}
1187-
1188-
static PyObject* makeFunctionType = PyObject_GetAttrString(internalsModule, "makeFunctionType");
1189-
1190-
if (!makeFunctionType) {
1191-
PyErr_SetString(PyExc_TypeError, "Internal error: couldn't find typed_python.internals.makeFunctionType");
1192-
return nullptr;
1193-
}
1125+
PyObject* makeFunctionType = staticPythonInstance("typed_python.internals", "makeFunctionType");
11941126

11951127
PyObjectStealer args(PyTuple_Pack(2, name, funcObj));
11961128
PyObjectStealer kwargs(PyDict_New());

‎typed_python/PyInstance.cpp‎

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -89,16 +89,12 @@ instance_ptr PyInstance::dataPtr() {
8989

9090
//static
9191
PyObject* PyInstance::undefinedBehaviorException() {
92-
static PyObject* module = ::internalsModule();
93-
static PyObject* t = PyObject_GetAttrString(module, "UndefinedBehaviorException");
94-
return t;
92+
return staticPythonInstance("typed_python.internals", "UndefinedBehaviorException");
9593
}
9694

9795
//static
9896
PyObject* PyInstance::nonTypesAcceptedAsTypes() {
99-
static PyObject* module = ::internalsModule();
100-
static PyObject* t = PyObject_GetAttrString(module, "_nonTypesAcceptedAsTypes");
101-
return t;
97+
return staticPythonInstance("typed_python.internals", "_nonTypesAcceptedAsTypes");
10298
}
10399

104100
// static
@@ -989,24 +985,6 @@ std::pair<Type*, instance_ptr> PyInstance::extractTypeAndPtrFrom(PyObject* obj)
989985
}
990986

991987

992-
PyObject* PyInstance::getInternalModuleMember(const char* name) {
993-
static PyObject* internalsModule = ::internalsModule();
994-
995-
if (!internalsModule) {
996-
PyErr_SetString(PyExc_TypeError, "Internal error: couldn't find typed_python.internals");
997-
return nullptr;
998-
}
999-
1000-
PyObject* result = PyObject_GetAttrString(internalsModule, name);
1001-
1002-
if (!result) {
1003-
PyErr_Format(PyExc_TypeError, "Internal error: couldn't find typed_python.internals.%s", name);
1004-
return nullptr;
1005-
}
1006-
1007-
return result;
1008-
}
1009-
1010988
//construct the base class that all actual type instances of a given TypeCategory descend from
1011989
PyTypeObject* PyInstance::allTypesBaseType() {
1012990
auto allocateBaseType = [&]() {
@@ -1165,7 +1143,9 @@ PyTypeObject* PyInstance::typeCategoryBaseType(Type::TypeCategory category) {
11651143
};
11661144

11671145
if (category == Type::TypeCategory::catClass) {
1168-
static PyTypeObject* classMetaclass = (PyTypeObject*)getInternalModuleMember("ClassMetaclass");
1146+
PyTypeObject* classMetaclass = (PyTypeObject*)staticPythonInstance(
1147+
"typed_python.internals", "ClassMetaclass"
1148+
);
11691149
((PyObject*)&types[category]->typeObj)->ob_type = incref(classMetaclass);
11701150
}
11711151

@@ -1300,7 +1280,9 @@ PyTypeObject* PyInstance::typeObjInternal(Type* inType) {
13001280
// if we are an instance of 'Class', we must explicitly set our Metaclass to internals.ClassMetaclass,
13011281
// so that when other classes inherit from us they also inherit our metaclass.
13021282
if (inType->getTypeCategory() == Type::TypeCategory::catClass) {
1303-
static PyTypeObject* classMetaclass = (PyTypeObject*)getInternalModuleMember("ClassMetaclass");
1283+
PyTypeObject* classMetaclass = (PyTypeObject*)staticPythonInstance(
1284+
"typed_python.internals", "ClassMetaclass"
1285+
);
13041286
((PyObject*)&types[inType]->typeObj)->ob_type = incref(classMetaclass);
13051287
}
13061288

‎typed_python/PyInstance.hpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -629,8 +629,6 @@ class PyInstance {
629629

630630
static PyBufferProcs* bufferProcs();
631631

632-
static PyObject* getInternalModuleMember(const char* name);
633-
634632
static PyTypeObject* allTypesBaseType();
635633

636634
static PyTypeObject* typeCategoryBaseType(Type::TypeCategory category);

‎typed_python/PyTemporaryReferenceTracer.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ bool PyTemporaryReferenceTracer::isLineNewStatement(PyObject* code, int line) {
2929
incref(code);
3030
auto& lineNumbers = codeObjectToExpressionLines[code];
3131

32-
static PyObject* internals = internalsModule();
32+
PyObject* internals = internalsModule();
3333

3434
PyObjectStealer res(
3535
PyObject_CallMethod(internals, "extractCodeObjectNewStatementLineNumbers", "O", code, NULL)

‎typed_python/PythonObjectOfTypeType.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -532,15 +532,17 @@ PythonObjectOfType* PythonObjectOfType::Make(PyTypeObject* pyType, PyObject* giv
532532
}
533533

534534
PythonObjectOfType* PythonObjectOfType::AnyPyObject() {
535-
static PyObject* module = internalsModule();
536-
static PyObject* t = PyObject_GetAttrString(module, "object");
535+
PyTypeObject* t = (PyTypeObject*)staticPythonInstance(
536+
"typed_python.internals", "object"
537+
);
537538

538539
return Make((PyTypeObject*)t);
539540
}
540541

541542
PythonObjectOfType* PythonObjectOfType::AnyPyType() {
542-
static PyObject* module = internalsModule();
543-
static PyObject* t = PyObject_GetAttrString(module, "type");
543+
PyTypeObject* t = (PyTypeObject*)staticPythonInstance(
544+
"typed_python.internals", "type"
545+
);
544546

545547
return Make((PyTypeObject*)t);
546548
}

‎typed_python/_runtime.cpp‎

Lines changed: 5 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -14,39 +14,9 @@
1414
#include <pythread.h>
1515

1616
PyObject* getRuntimeSingleton() {
17-
assertHoldingTheGil();
18-
19-
static PyObject* pyRuntimeModule = runtimeModule();
20-
21-
if (!pyRuntimeModule) {
22-
if (!PyErr_Occurred()) {
23-
PyErr_Format(PyExc_RuntimeError, "Internal error: couldn't find typed_python.compiler.runtime");
24-
}
25-
throw PythonExceptionSet();
26-
}
27-
28-
static PyObject* runtimeClass = PyObject_GetAttrString(pyRuntimeModule, "Runtime");
29-
30-
if (!runtimeClass) {
31-
if (!PyErr_Occurred()) {
32-
PyErr_Format(PyExc_RuntimeError, "Internal error: couldn't find typed_python.compiler.runtime.Runtime");
33-
}
34-
throw PythonExceptionSet();
35-
}
36-
37-
static PyObject* singleton = PyObject_CallMethod(runtimeClass, "singleton", "");
38-
39-
if (!singleton) {
40-
if (!PyErr_Occurred()) {
41-
PyErr_Format(
42-
PyExc_RuntimeError,
43-
"Internal error: couldn't call typed_python.compiler.runtime.Runtime.singleton"
44-
);
45-
}
46-
throw PythonExceptionSet();
47-
}
48-
49-
return singleton;
17+
return staticPythonInstance(
18+
"typed_python.compiler.runtime", "Runtime.singleton()"
19+
);
5020
}
5121

5222
// Note: extern C identifiers are distinguished only up to 32 characters
@@ -1247,9 +1217,7 @@ extern "C" {
12471217
PythonObjectOfType::layout_type* np_builtin_pyobj_by_name(const char* utf8_name) {
12481218
PyEnsureGilAcquired getTheGil;
12491219

1250-
static PyObject* module = builtinsModule();
1251-
1252-
return PythonObjectOfType::createLayout(PyObject_GetAttrString(module, utf8_name));
1220+
return PythonObjectOfType::createLayout(PyObject_GetAttrString(builtinsModule(), utf8_name));
12531221
}
12541222

12551223
PythonObjectOfType::layout_type* nativepython_runtime_get_pyobj_None() {
@@ -2715,9 +2683,7 @@ extern "C" {
27152683
void np_raise_exception_fastpath(const char* message, const char* exceptionTypeName) {
27162684
PyEnsureGilAcquired getTheGil;
27172685

2718-
PyObject* module = builtinsModule();
2719-
2720-
PyObject* excType = PyObject_GetAttrString(module, exceptionTypeName);
2686+
PyObject* excType = PyObject_GetAttrString(builtinsModule(), exceptionTypeName);
27212687
if (!excType) {
27222688
return;
27232689
}

0 commit comments

Comments
 (0)