Skip to content

Commit a83347c

Browse files
steve-ssteve-s
committed
[GR-60268] Test context cancellation with multithreading.
PullRequest: graalpython/3598
2 parents 086558b + 463009b commit a83347c

File tree

13 files changed

+400
-106
lines changed

13 files changed

+400
-106
lines changed

‎graalpython/com.oracle.graal.python.cext/src/pystate.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,14 @@ PyGILState_Release(PyGILState_STATE oldstate)
181181
if (TRUFFLE_CONTEXT) {
182182
graalpy_gilstate_counter--;
183183
if (graalpy_gilstate_counter == 0 && graalpy_attached_thread) {
184+
GraalPyTruffleBeforeThreadDetach();
184185
(*TRUFFLE_CONTEXT)->detachCurrentThread(TRUFFLE_CONTEXT);
185186
graalpy_attached_thread = 0;
186187
/*
187-
* The thread state on the Java-side can get garbage collected after the thread is detached,
188-
* so we need to make sure to fetch a fresh pointer the next time we attach.
188+
* The thread state on the Java-side is cleared in GraalPyTruffleBeforeThreadDetach.
189+
* As part of that the tstate_current pointer should have been set to NULL to make
190+
* sure to fetch a fresh pointer the next time we attach. Just to be sure, we clear
191+
* it here too:
189192
*/
190193
tstate_current = NULL;
191194
}

‎graalpython/com.oracle.graal.python.test.integration/src/com/oracle/graal/python/test/integration/advanced/MultiContextTest.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,16 +58,13 @@ public void testSharingWithMemoryview() {
5858
}
5959

6060
@Test
61-
public void testSharingWithCpythonSre() {
62-
// With the default value of NativeModules=true, this test is going to use the NFI C API
63-
// backend for the first context, but then it is going to use the Sulong backend for
64-
// consecutive contexts (as long as this is the only test that executes native code,
65-
// which it seems to be). This is why we need "sulong:SULONG_NATIVE" among the
66-
// dependencies for GRAALPYTHON_UNIT_TESTS distribution, and
61+
public void testSharingWithCpythonSreAndLLVM() {
62+
// This test is going to use the Sulong backend.This is why we need "sulong:SULONG_NATIVE"
63+
// among the dependencies for GRAALPYTHON_UNIT_TESTS distribution, and
6764
// org.graalvm.polyglot:llvm-community dependency in the pom.xml
6865
Engine engine = Engine.newBuilder().build();
6966
for (int i = 0; i < 10; i++) {
70-
try (Context context = newContext(engine)) {
67+
try (Context context = newContextWithNativeModulesFalse(engine)) {
7168
context.eval("python", "import _cpython_sre\nassert _cpython_sre.ascii_tolower(88) == 120\n");
7269
}
7370
}
@@ -93,4 +90,8 @@ public void testTryCatch() {
9390
private static Context newContext(Engine engine) {
9491
return Context.newBuilder().allowExperimentalOptions(true).allowAllAccess(true).engine(engine).build();
9592
}
93+
94+
private static Context newContextWithNativeModulesFalse(Engine engine) {
95+
return Context.newBuilder().allowExperimentalOptions(true).allowAllAccess(true).engine(engine).option("python.NativeModules", "false").build();
96+
}
9697
}

‎graalpython/com.oracle.graal.python.test.integration/src/com/oracle/graal/python/test/integration/advanced/NativeExtTest.java

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/*
2+
* Copyright (c) 2017, 2024, 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.test.integration.advanced;
42+
43+
import java.util.concurrent.CountDownLatch;
44+
45+
import org.graalvm.polyglot.Context;
46+
import org.graalvm.polyglot.Engine;
47+
import org.graalvm.polyglot.PolyglotException;
48+
import org.junit.Assert;
49+
import org.junit.Test;
50+
51+
/**
52+
* This should be the only test using native extensions. We must test everything in one long test,
53+
* because we cannot create multiple contexts that would load native extensions.
54+
*/
55+
public class NativeExtTest {
56+
@Test
57+
public void testSharingErrorWithCpythonSre() throws InterruptedException {
58+
// The first context is the one that will use native extensions
59+
Engine engine = Engine.newBuilder().build();
60+
Context cextContext = newContext(engine);
61+
try {
62+
cextContext.eval("python", "import _cpython_sre\nassert _cpython_sre.ascii_tolower(88) == 120\n");
63+
64+
// Check that second context that tries to load native extension fails
65+
try (Context secondCtx = newContext(engine)) {
66+
try {
67+
secondCtx.eval("python", "import _cpython_sre\nassert _cpython_sre.ascii_tolower(88) == 120\n");
68+
} catch (PolyglotException ex) {
69+
Assert.assertTrue(ex.getMessage(), ex.getMessage().contains("Option python.NativeModules is set to 'true' and a second GraalPy context attempted"));
70+
}
71+
}
72+
73+
// To test cancellation we are going to spawn some Python threads...
74+
ShutdownTest.asyncStartPythonThreadsThatSleep(cextContext);
75+
76+
// A java thread that attaches to GraalPy and finishes before the cancellation
77+
CountDownLatch latch = new CountDownLatch(1);
78+
new Thread(() -> {
79+
cextContext.eval("python", "import _cpython_sre\nassert _cpython_sre.ascii_tolower(88) == 120\n");
80+
latch.countDown();
81+
}).start();
82+
latch.await();
83+
84+
// A java thread that attaches to GraalPy and sleeps forever in Java
85+
CountDownLatch latch2 = new CountDownLatch(1);
86+
new Thread(() -> {
87+
cextContext.eval("python", "import _cpython_sre\nassert _cpython_sre.ascii_tolower(88) == 120\n");
88+
latch2.countDown();
89+
try {
90+
Thread.sleep(100000000);
91+
} catch (InterruptedException e) {
92+
// expected
93+
}
94+
}).start();
95+
latch2.await();
96+
97+
} finally {
98+
cextContext.close(true);
99+
}
100+
}
101+
102+
private static Context newContext(Engine engine) {
103+
return Context.newBuilder().allowExperimentalOptions(true).allowAllAccess(true).engine(engine).build();
104+
}
105+
}

‎graalpython/com.oracle.graal.python.test.integration/src/com/oracle/graal/python/test/integration/advanced/ShutdownTest.java

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
/*
2+
* Copyright (c) 2017, 2024, 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.test.integration.advanced;
42+
43+
import org.graalvm.polyglot.Context;
44+
import org.graalvm.polyglot.PolyglotException;
45+
import org.junit.Assert;
46+
import org.junit.Test;
47+
48+
import com.oracle.graal.python.test.integration.PythonTests;
49+
50+
import java.util.concurrent.CountDownLatch;
51+
import java.util.concurrent.TimeUnit;
52+
import java.util.concurrent.atomic.AtomicReference;
53+
54+
// See also NativeExtTest
55+
public class ShutdownTest extends PythonTests {
56+
@Test
57+
public void testCloseWithBackgroundThreadsRunningSucceeds() {
58+
Context context = createContext();
59+
try {
60+
loadNativeExtension(context);
61+
asyncStartPythonThreadsThatSleep(context);
62+
} finally {
63+
context.close(true);
64+
}
65+
}
66+
67+
@Test
68+
public void testCloseFromAnotherThreadThrowsCancelledEx() {
69+
Context context = createContext();
70+
PolyglotException thrownEx = null;
71+
try {
72+
loadNativeExtension(context);
73+
asyncStartPythonThreadsThatSleep(context);
74+
CountDownLatch latch = new CountDownLatch(1);
75+
new Thread(() -> {
76+
try {
77+
latch.await();
78+
Thread.sleep(10);
79+
} catch (InterruptedException e) {
80+
throw new RuntimeException(e);
81+
}
82+
// close from another thread
83+
context.close(true);
84+
}).start();
85+
countDownLatchAndSleepInPython(context, latch);
86+
} catch (PolyglotException ex) {
87+
thrownEx = ex;
88+
} finally {
89+
context.close(true);
90+
Assert.assertNotNull("PolyglotException was not thrown upon cancellation ", thrownEx);
91+
Assert.assertTrue(thrownEx.toString(), thrownEx.isCancelled());
92+
}
93+
}
94+
95+
@Test
96+
public void testJavaThreadGetsCancelledException() throws InterruptedException {
97+
Context context = createContext();
98+
AtomicReference<PolyglotException> thrownEx = new AtomicReference<>();
99+
CountDownLatch gotException = new CountDownLatch(1);
100+
try {
101+
asyncStartPythonThreadsThatSleep(context);
102+
CountDownLatch latch = new CountDownLatch(1);
103+
new Thread(() -> {
104+
try {
105+
// this thread will get stuck sleeping in python
106+
countDownLatchAndSleepInPython(context, latch);
107+
} catch (PolyglotException ex) {
108+
thrownEx.set(ex);
109+
gotException.countDown();
110+
}
111+
}).start();
112+
try {
113+
loadNativeExtension(context);
114+
latch.await();
115+
Thread.sleep(10); // make sure the other thread really starts sleeping
116+
} catch (InterruptedException e) {
117+
throw new RuntimeException(e);
118+
}
119+
} finally {
120+
context.close(true);
121+
}
122+
Assert.assertTrue("other thread did not get any exception in time limit", gotException.await(2, TimeUnit.SECONDS));
123+
Assert.assertNotNull("PolyglotException was not thrown upon cancellation ", thrownEx.get());
124+
Assert.assertTrue(thrownEx.toString(), thrownEx.get().isCancelled());
125+
}
126+
127+
@Test
128+
public void testJavaThreadNotExecutingPythonAnymore() throws InterruptedException {
129+
Context context = createContext();
130+
var javaThreadDone = new CountDownLatch(1);
131+
var javaThreadCanEnd = new CountDownLatch(1);
132+
var javaThread = new Thread(() -> {
133+
Assert.assertEquals(42, context.eval("python", "42").asInt());
134+
javaThreadDone.countDown();
135+
try {
136+
javaThreadCanEnd.await();
137+
} catch (InterruptedException e) {
138+
throw new RuntimeException(e);
139+
}
140+
});
141+
AtomicReference<Throwable> uncaughtEx = new AtomicReference<>();
142+
javaThread.setUncaughtExceptionHandler((t, e) -> uncaughtEx.set(e));
143+
try {
144+
javaThread.start();
145+
loadNativeExtension(context);
146+
javaThreadDone.await();
147+
} finally {
148+
// we can close although the Java thread is still running
149+
context.close(true);
150+
}
151+
javaThreadCanEnd.countDown();
152+
javaThread.join();
153+
Assert.assertNull(uncaughtEx.get());
154+
}
155+
156+
private static Context createContext() {
157+
return Context.newBuilder().allowExperimentalOptions(true).allowAllAccess(true).option("python.NativeModules", "false").build();
158+
}
159+
160+
private static void loadNativeExtension(Context context) {
161+
context.eval("python", "import _cpython_sre\nassert _cpython_sre.ascii_tolower(88) == 120\n");
162+
}
163+
164+
public static void asyncStartPythonThreadsThatSleep(Context context) {
165+
for (int i = 0; i < 10; i++) {
166+
context.eval("python", "import threading; import time; threading.Thread(target=lambda: time.sleep(10000)).start()");
167+
}
168+
}
169+
170+
private static void countDownLatchAndSleepInPython(Context context, CountDownLatch latch) {
171+
context.eval("python", "def run(latch): import time; latch.countDown(); time.sleep(100000)");
172+
context.getBindings("python").getMember("run").execute(latch);
173+
}
174+
}

‎graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/nodes/MemMoveNodeTests.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,17 @@
5757
import org.junit.Before;
5858
import org.junit.Test;
5959

60+
import java.util.Map;
61+
6062
public class MemMoveNodeTests {
6163

6264
private GilNode.UncachedAcquire gil;
6365

6466
@Before
6567
public void setUp() {
66-
PythonTests.enterContext();
68+
PythonTests.enterContext(Map.of("python.NativeModules", "false"), new String[0]);
6769
this.gil = GilNode.uncachedAcquire();
68-
CApiContext.ensureCapiWasLoaded();
70+
CApiContext.ensureCapiWasLoaded("internal");
6971
}
7072

7173
@After

‎graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_thread.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,21 +87,20 @@ def test_register_new_thread(self):
8787
name="TestThread",
8888
includes="#include <pthread.h>",
8989
code=r'''
90-
void* thread_entrypoint(void* arg) {
90+
void test_attach_detach(PyObject *callable) {
9191
// This check is important not just to check that the function works without the thread attached,
9292
// but also because the thread attaching logic in it can break the following PyGILState_Ensure call
9393
if (PyGILState_Check()) {
9494
PyErr_SetString(PyExc_RuntimeError, "Thread shouldn't be holding the GIL at this point");
9595
PyErr_WriteUnraisable(NULL);
96-
return NULL;
96+
return;
9797
}
98-
PyObject* callable = (PyObject*)arg;
9998
PyGILState_STATE gstate;
10099
gstate = PyGILState_Ensure();
101100
if (!PyGILState_Check()) {
102101
PyErr_SetString(PyExc_RuntimeError, "GIL not acquired");
103102
PyErr_WriteUnraisable(NULL);
104-
return NULL;
103+
return;
105104
}
106105
if (!PyObject_CallNoArgs(callable)) {
107106
PyErr_WriteUnraisable(callable);
@@ -113,8 +112,12 @@ def test_register_new_thread(self):
113112
if (PyGILState_Check()) {
114113
PyErr_SetString(PyExc_RuntimeError, "GIL not released");
115114
PyErr_WriteUnraisable(NULL);
116-
return NULL;
117115
}
116+
}
117+
void* thread_entrypoint(void* arg) {
118+
test_attach_detach((PyObject*)arg);
119+
test_attach_detach((PyObject*)arg);
120+
test_attach_detach((PyObject*)arg);
118121
return NULL;
119122
}
120123
PyObject* run_in_thread(PyObject* self, PyObject* callable) {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/PythonLanguage.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1028,7 +1028,7 @@ protected void initializeThread(PythonContext context, Thread thread) {
10281028

10291029
@Override
10301030
protected void disposeThread(PythonContext context, Thread thread) {
1031-
context.disposeThread(thread);
1031+
context.disposeThread(thread, false);
10321032
}
10331033

10341034
public Shape getEmptyShape() {

0 commit comments

Comments
 (0)