Skip to content

Commit f4b7ca3

Browse files
steve-ssteve-s
committed
[GR-40637] [GR-68713] Fix: multi-threaded module importing.
PullRequest: graalpython/3951
2 parents 088ed35 + 18aca84 commit f4b7ca3

File tree

12 files changed

+461
-146
lines changed

12 files changed

+461
-146
lines changed

‎graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/cext/test/MultithreadedImportTestBase.java‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
/*
2+
* Copyright (c) 2025, 2025, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* The Universal Permissive License (UPL), Version 1.0
6+
*
7+
* Subject to the condition set forth below, permission is hereby granted to any
8+
* person obtaining a copy of this software, associated documentation and/or
9+
* data (collectively the "Software"), free of charge and under any and all
10+
* copyright rights in the Software, and any and all patent rights owned or
11+
* freely licensable by each licensor hereunder covering either (i) the
12+
* unmodified Software as contributed to or provided by such licensor, or (ii)
13+
* the Larger Works (as defined below), to deal in both
14+
*
15+
* (a) the Software, and
16+
*
17+
* (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
18+
* one is included with the Software each a "Larger Work" to which the Software
19+
* is contributed by such licensors),
20+
*
21+
* without restriction, including without limitation the rights to copy, create
22+
* derivative works of, display, perform, and distribute the Software and make,
23+
* use, sell, offer for sale, import, export, have made, and have sold the
24+
* Software and the Larger Work(s), and to sublicense the foregoing rights on
25+
* either these or other terms.
26+
*
27+
* This license is subject to the following condition:
28+
*
29+
* The above copyright notice and either this complete permission notice or at a
30+
* minimum a reference to the UPL must be included in all copies or substantial
31+
* portions of the Software.
32+
*
33+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
34+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
35+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
36+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
37+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
38+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
39+
* SOFTWARE.
40+
*/
41+
package com.oracle.graal.python.cext.test;
42+
43+
import java.lang.management.ManagementFactory;
44+
import java.lang.management.ThreadInfo;
45+
import java.lang.management.ThreadMXBean;
46+
import java.util.ArrayList;
47+
import java.util.concurrent.ExecutorService;
48+
import java.util.concurrent.Executors;
49+
import java.util.concurrent.Future;
50+
import java.util.concurrent.TimeUnit;
51+
import java.util.concurrent.TimeoutException;
52+
import java.util.stream.Collectors;
53+
54+
import org.graalvm.polyglot.Context;
55+
import org.junit.Assert;
56+
57+
public class MultithreadedImportTestBase {
58+
private static final int ITERATIONS_LIMIT = 10;
59+
60+
// This test should be executed in its own process. It tests that there are no deadlocks. We do
61+
// not want to wait for the gate job timeout and timeout the test ourselves after shorter period
62+
// of time, but for that we must not call ExecutorService#close, which would be waiting for the
63+
// threads to finish
64+
@SuppressWarnings("resource")
65+
static void multithreadedImportTest(int numberOfThreads, Context context) {
66+
try {
67+
ExecutorService executor = Executors.newFixedThreadPool(numberOfThreads);
68+
var tasks = new ArrayList<Future<?>>();
69+
for (String pkg : PACKAGES.trim().split("\n")) {
70+
log("Starting import: %s", pkg);
71+
tasks.add(executor.submit(() -> {
72+
log("Importing %s on thread %s", pkg, Thread.currentThread());
73+
context.eval("python", "import " + pkg);
74+
}));
75+
}
76+
77+
int iteration = 0;
78+
while (!tasks.isEmpty() && iteration++ < ITERATIONS_LIMIT) {
79+
log("Iteration %s, looping over remaining %d unfinished tasks", iteration, tasks.size());
80+
var finishedTasks = tasks.stream().filter(task -> {
81+
try {
82+
task.get(1000, TimeUnit.MILLISECONDS);
83+
return true;
84+
} catch (TimeoutException timeoutEx) {
85+
return false;
86+
} catch (Exception ex) {
87+
log("Caught exception: %s", ex);
88+
throw new RuntimeException(ex);
89+
}
90+
}).collect(Collectors.toCollection(ArrayList::new));
91+
tasks.removeAll(finishedTasks);
92+
}
93+
94+
if (tasks.isEmpty()) {
95+
executor.shutdown();
96+
} else {
97+
// otherwise do not wait for the threads to finish, just dump them and continue to
98+
// fail the assertion below
99+
try {
100+
System.out.println("There are unfinished tasks. This failure is inherently transient. " +
101+
"Please report any failure. Thread dump is below if available:");
102+
ThreadMXBean threadMxBean = ManagementFactory.getThreadMXBean();
103+
for (ThreadInfo threadInfo : threadMxBean.dumpAllThreads(true, true, 20)) {
104+
System.out.print(threadInfo.toString());
105+
}
106+
} catch (UnsupportedOperationException ignored) {
107+
}
108+
}
109+
110+
Assert.assertTrue("Unfinished tasks", tasks.isEmpty());
111+
log("DONE: %s", MultithreadedImportTestBase.class.getSimpleName());
112+
} finally {
113+
context.close(true);
114+
}
115+
}
116+
117+
private static void log(String fmt, Object... args) {
118+
System.out.printf(fmt + "%n", args);
119+
}
120+
121+
private static final String PACKAGES = """
122+
csv
123+
configparser
124+
tomllib
125+
hashlib
126+
os
127+
_testcapi
128+
io
129+
time
130+
logging
131+
ctypes
132+
argparse
133+
_sqlite3
134+
_cpython_sre
135+
threading
136+
multiprocessing
137+
sched
138+
contextvars
139+
json
140+
pyexpat
141+
base64
142+
html
143+
locale
144+
shlex
145+
venv
146+
ast
147+
re
148+
difflib
149+
zlib
150+
""";
151+
}

‎graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/cext/test/MultithreadedImportTestJava.java‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/*
2+
* Copyright (c) 2025, 2025, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* The Universal Permissive License (UPL), Version 1.0
6+
*
7+
* Subject to the condition set forth below, permission is hereby granted to any
8+
* person obtaining a copy of this software, associated documentation and/or
9+
* data (collectively the "Software"), free of charge and under any and all
10+
* copyright rights in the Software, and any and all patent rights owned or
11+
* freely licensable by each licensor hereunder covering either (i) the
12+
* unmodified Software as contributed to or provided by such licensor, or (ii)
13+
* the Larger Works (as defined below), to deal in both
14+
*
15+
* (a) the Software, and
16+
*
17+
* (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
18+
* one is included with the Software each a "Larger Work" to which the Software
19+
* is contributed by such licensors),
20+
*
21+
* without restriction, including without limitation the rights to copy, create
22+
* derivative works of, display, perform, and distribute the Software and make,
23+
* use, sell, offer for sale, import, export, have made, and have sold the
24+
* Software and the Larger Work(s), and to sublicense the foregoing rights on
25+
* either these or other terms.
26+
*
27+
* This license is subject to the following condition:
28+
*
29+
* The above copyright notice and either this complete permission notice or at a
30+
* minimum a reference to the UPL must be included in all copies or substantial
31+
* portions of the Software.
32+
*
33+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
34+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
35+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
36+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
37+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
38+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
39+
* SOFTWARE.
40+
*/
41+
package com.oracle.graal.python.cext.test;
42+
43+
import static com.oracle.graal.python.cext.test.MultithreadedImportTestBase.multithreadedImportTest;
44+
45+
import org.graalvm.polyglot.Context;
46+
import org.junit.Test;
47+
48+
public class MultithreadedImportTestJava {
49+
@Test
50+
public void testImportOnMultipleThreads() {
51+
Context context = Context.newBuilder().allowAllAccess(true).option("python.PosixModuleBackend", "java").build();
52+
multithreadedImportTest(8, context);
53+
}
54+
}

‎graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/cext/test/MultithreadedImportTestNative.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
* Copyright (c) 2025, 2025, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* The Universal Permissive License (UPL), Version 1.0
6+
*
7+
* Subject to the condition set forth below, permission is hereby granted to any
8+
* person obtaining a copy of this software, associated documentation and/or
9+
* data (collectively the "Software"), free of charge and under any and all
10+
* copyright rights in the Software, and any and all patent rights owned or
11+
* freely licensable by each licensor hereunder covering either (i) the
12+
* unmodified Software as contributed to or provided by such licensor, or (ii)
13+
* the Larger Works (as defined below), to deal in both
14+
*
15+
* (a) the Software, and
16+
*
17+
* (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
18+
* one is included with the Software each a "Larger Work" to which the Software
19+
* is contributed by such licensors),
20+
*
21+
* without restriction, including without limitation the rights to copy, create
22+
* derivative works of, display, perform, and distribute the Software and make,
23+
* use, sell, offer for sale, import, export, have made, and have sold the
24+
* Software and the Larger Work(s), and to sublicense the foregoing rights on
25+
* either these or other terms.
26+
*
27+
* This license is subject to the following condition:
28+
*
29+
* The above copyright notice and either this complete permission notice or at a
30+
* minimum a reference to the UPL must be included in all copies or substantial
31+
* portions of the Software.
32+
*
33+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
34+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
35+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
36+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
37+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
38+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
39+
* SOFTWARE.
40+
*/
41+
package com.oracle.graal.python.cext.test;
42+
43+
import org.graalvm.polyglot.Context;
44+
import org.junit.Test;
45+
46+
public class MultithreadedImportTestNative extends MultithreadedImportTestBase {
47+
@Test
48+
public void testImportOnMultipleThreads() {
49+
Context context = Context.newBuilder().allowAllAccess(true).option("python.PosixModuleBackend", "native").build();
50+
multithreadedImportTest(32, context);
51+
}
52+
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/util/Cache.java‎ renamed to ‎graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/cext/test/package-info.java‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2025, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -38,14 +38,9 @@
3838
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
3939
* SOFTWARE.
4040
*/
41-
package com.oracle.graal.python.util;
4241

43-
public interface Cache<K, V> {
44-
V get(K key);
45-
46-
V put(K key, V value);
47-
48-
void clear();
49-
50-
int size();
51-
}
42+
/**
43+
* Package for tests that need to run in a separate process due to C extensions usage. Add a
44+
* dedicated TestConfig into mx_graalpython#punittest for each test in this package.
45+
*/
46+
package com.oracle.graal.python.cext.test;

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/ImpModuleBuiltins.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ public abstract static class AcquireLock extends PythonBuiltinNode {
213213
public Object run(@Cached GilNode gil) {
214214
gil.release(true);
215215
try {
216-
TruffleSafepoint.setBlockedThreadInterruptible(this, ReentrantLock::lock, getContext().getImportLock());
216+
TruffleSafepoint.setBlockedThreadInterruptible(this, ReentrantLock::lockInterruptibly, getContext().getImportLock());
217217
} finally {
218218
gil.acquire();
219219
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/StructModuleBuiltins.java‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,12 @@ public void postInitialize(Python3Core core) {
7878
structModule.setModuleState(cache);
7979
}
8080

81-
protected static PStruct getStruct(PythonModule structModule, Object format, StructBuiltins.ConstructStructNode constructStructNode) {
81+
protected static PStruct getStruct(Node location, PythonModule structModule, Object format, StructBuiltins.ConstructStructNode constructStructNode) {
8282
LRUStructCache cache = structModule.getModuleState(LRUStructCache.class);
83-
PStruct pStruct = cache.get(format);
83+
PStruct pStruct = cache.get(location, format);
8484
if (pStruct == null) {
8585
pStruct = constructStructNode.execute(format);
86-
cache.put(format, pStruct);
86+
cache.put(location, format, pStruct);
8787
}
8888
return pStruct;
8989
}
@@ -94,8 +94,8 @@ protected static PStruct getStruct(PythonModule structModule, Object format, Str
9494
abstract static class GetStructNode extends PNodeWithContext {
9595
abstract PStruct execute(Node inliningTarget, PythonModule module, Object format, StructBuiltins.ConstructStructNode constructStructNode);
9696

97-
protected PStruct getStructInternal(PythonModule module, Object format, StructBuiltins.ConstructStructNode constructStructNode) {
98-
return getStruct(module, format, constructStructNode);
97+
protected PStruct getStructInternal(Node location, PythonModule module, Object format, StructBuiltins.ConstructStructNode constructStructNode) {
98+
return getStruct(location, module, format, constructStructNode);
9999
}
100100

101101
protected boolean eq(TruffleString s1, TruffleString s2, TruffleString.EqualNode eqNode) {
@@ -107,7 +107,7 @@ protected boolean eq(TruffleString s1, TruffleString s2, TruffleString.EqualNode
107107
static PStruct doCachedString(PythonModule module, TruffleString format, StructBuiltins.ConstructStructNode constructStructNode,
108108
@Cached("format") TruffleString cachedFormat,
109109
@Cached TruffleString.EqualNode eqNode,
110-
@Cached(value = "getStructInternal(module, format, constructStructNode)", weak = true) PStruct cachedStruct) {
110+
@Cached(value = "getStructInternal($node, module, format, constructStructNode)", weak = true) PStruct cachedStruct) {
111111
return cachedStruct;
112112
}
113113

@@ -116,13 +116,14 @@ static PStruct doCachedString(PythonModule module, TruffleString format, StructB
116116
static PStruct doCachedBytes(PythonModule module, PBytes format, StructBuiltins.ConstructStructNode constructStructNode,
117117
@CachedLibrary("format") PythonBufferAccessLibrary bufferLib,
118118
@Cached(value = "bufferLib.getCopiedByteArray(format)", dimensions = 1) byte[] cachedFormat,
119-
@Cached(value = "getStructInternal(module, format, constructStructNode)", weak = true) PStruct cachedStruct) {
119+
@Cached(value = "getStructInternal($node, module, format, constructStructNode)", weak = true) PStruct cachedStruct) {
120120
return cachedStruct;
121121
}
122122

123123
@Specialization(replaces = {"doCachedString", "doCachedBytes"})
124-
static PStruct doGeneric(PythonModule module, Object format, StructBuiltins.ConstructStructNode constructStructNode) {
125-
return getStruct(module, format, constructStructNode);
124+
static PStruct doGeneric(PythonModule module, Object format, StructBuiltins.ConstructStructNode constructStructNode,
125+
@Bind Node location) {
126+
return getStruct(location, module, format, constructStructNode);
126127
}
127128
}
128129

@@ -241,7 +242,7 @@ abstract static class ClearCacheNode extends PythonUnaryBuiltinNode {
241242
@Specialization
242243
Object clearCache(PythonModule self) {
243244
LRUStructCache cache = self.getModuleState(LRUStructCache.class);
244-
cache.clear();
245+
cache.clear(this);
245246
return PNone.NONE;
246247
}
247248
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextBuiltins.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1887,10 +1887,18 @@ private static PythonManagedClass lookupBuiltinTypeWithName(PythonContext contex
18871887
} else {
18881888
String module = name.substring(0, index);
18891889
name = name.substring(index + 1);
1890-
Object moduleObject = core.lookupBuiltinModule(toTruffleStringUncached(module));
1890+
TruffleString tsModule = toTruffleStringUncached(module);
1891+
Object moduleObject = core.lookupBuiltinModule(tsModule);
18911892
if (moduleObject == null) {
1892-
moduleObject = AbstractImportNode.importModule(toTruffleStringUncached(module));
1893+
moduleObject = AbstractImportNode.lookupImportedModule(context, tsModule);
1894+
if (moduleObject == null) {
1895+
throw CompilerDirectives.shouldNotReachHere(String.format(
1896+
"Module '%s' is needed during C API initialization, but was not imported prior to the initialization in ensureCapiWasLoaded. This is an internal error in GraalPy.",
1897+
module));
1898+
}
18931899
}
1900+
// Assumption: builtin modules' tp_getattro is well-behaved and just reads the
1901+
// attribute, there is no locking or blocking inside
18941902
Object attribute = PyObjectGetAttr.getUncached().execute(null, moduleObject, toTruffleStringUncached(name));
18951903
if (attribute != PNone.NO_VALUE) {
18961904
if (attribute instanceof PythonBuiltinClassType builtinType) {

0 commit comments

Comments
 (0)