Skip to content

Commit 7ead338

Browse files
committed
[GR-30335] Use safepoints for GIL and async actions
PullRequest: graalpython/1728
2 parents d7f09b0 + a142428 commit 7ead338

File tree

25 files changed

+386
-340
lines changed

25 files changed

+386
-340
lines changed

docs/contributor/IMPLEMENTATION_DETAILS.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,3 +248,28 @@ we are always running single threaded or always own the GIL already when we get
248248
to a specific message we never emit a boundary call to lock and unlock the
249249
GIL. This implies that we may deopt in some places if, after compiling some
250250
code, we later start a second thread.
251+
252+
## Threading
253+
254+
As explained above, we use a GIL to prevent parallel execution of Python code. A
255+
timer is running the interrupts threads periodically to relinquish the GIL and
256+
give other threads a chance to run. This preemption is prohibited in most C
257+
extension code, however, since the assumption in C extensions written for
258+
CPython is that the GIL will not be relinquished while executing the C code and
259+
many C extensions are not written to reentrant.
260+
261+
So, commonly at any given time there is only one Graal Python thread executing
262+
and all the other threads are waiting to acquire the GIL. If the Python
263+
interpreter shuts down, there are two sets of threads we need to deal with:
264+
"well-behaved" threads created using the `threading` module are interrupted and
265+
joined using the `threading` module's `shutdown` function. Daemon threads or
266+
threads created using the internal Python `_thread` module cannot be joined in
267+
this way. For those threads, we invalidate their `PythonThreadState` (a
268+
thread-local data structure) and use the Java `Thread#interrupt` method to
269+
interrupt their waiting on the GIL. This exception is handled by checking if the
270+
thread state has been invalidated and if so, just exit the thread gracefully.
271+
272+
For embedders, it may be important to be able to interrupt Python threads by
273+
other means. We use the TruffleSafepoint mechanism to mark our threads waiting
274+
to acquire the GIL as blocked for the purpose of safepoints. The Truffle
275+
safepoint action mechanism can thus be used to kill threads waiting on the GIL.

docs/user/Packages.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,4 @@ Note that to try extensions on GraalVM's Python runtime, you have to download, b
3636

3737
### Using `pip`
3838

39-
The `pip` package installer is available and working in a `venv`, but there is no support for SSL yet.
40-
This means you can install packages from PyPI if you use an HTTP mirror, and you can install local packages.
39+
The `pip` package installer is available and working when using a `venv`.

graalpython/com.oracle.graal.python.test/src/tests/unittest_tags/test_pickle.txt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
*graalpython.lib-python.3.test.test_pickle.CompatPickleTests.test_reverse_name_mapping
88
*graalpython.lib-python.3.test.test_pickle.InMemoryPickleTests.test_appends_on_non_lists
99
*graalpython.lib-python.3.test.test_pickle.InMemoryPickleTests.test_attribute_name_interning
10-
*graalpython.lib-python.3.test.test_pickle.InMemoryPickleTests.test_bad_getattr
1110
*graalpython.lib-python.3.test.test_pickle.InMemoryPickleTests.test_bad_mark
1211
*graalpython.lib-python.3.test.test_pickle.InMemoryPickleTests.test_bad_newobj
1312
*graalpython.lib-python.3.test.test_pickle.InMemoryPickleTests.test_bad_newobj_ex
@@ -167,7 +166,6 @@
167166
*graalpython.lib-python.3.test.test_pickle.PyPicklerHookTests.test_reducer_override_no_reference_cycle
168167
*graalpython.lib-python.3.test.test_pickle.PyPicklerTests.test_appends_on_non_lists
169168
*graalpython.lib-python.3.test.test_pickle.PyPicklerTests.test_attribute_name_interning
170-
*graalpython.lib-python.3.test.test_pickle.PyPicklerTests.test_bad_getattr
171169
*graalpython.lib-python.3.test.test_pickle.PyPicklerTests.test_buffer_callback_error
172170
*graalpython.lib-python.3.test.test_pickle.PyPicklerTests.test_buffers_error
173171
*graalpython.lib-python.3.test.test_pickle.PyPicklerTests.test_buffers_numpy

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,13 @@ protected CallTarget parse(ParsingRequest request) {
311311
}
312312
RootNode root = doParse(context, source, 0);
313313
if (root instanceof PRootNode) {
314-
((PRootNode) root).triggerDeprecationWarnings();
314+
GilNode gil = GilNode.getUncached();
315+
boolean wasAcquired = gil.acquire(context, root);
316+
try {
317+
((PRootNode) root).triggerDeprecationWarnings();
318+
} finally {
319+
gil.release(context, wasAcquired);
320+
}
315321
}
316322
if (core.isInitialized()) {
317323
return PythonUtils.getOrCreateCallTarget(new TopLevelExceptionHandler(this, root, source));

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/FaulthandlerModuleBuiltins.java

Lines changed: 10 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -43,40 +43,31 @@
4343
import java.io.PrintWriter;
4444
import java.util.List;
4545
import java.util.Map;
46-
import java.util.WeakHashMap;
4746

4847
import com.oracle.graal.python.annotations.ArgumentClinic;
4948
import com.oracle.graal.python.builtins.Builtin;
5049
import com.oracle.graal.python.builtins.CoreFunctions;
5150
import com.oracle.graal.python.builtins.PythonBuiltins;
5251
import com.oracle.graal.python.builtins.objects.PNone;
53-
import com.oracle.graal.python.builtins.objects.module.PythonModule;
5452
import com.oracle.graal.python.builtins.objects.object.PythonObjectLibrary;
5553
import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
5654
import com.oracle.graal.python.nodes.function.builtins.PythonClinicBuiltinNode;
5755
import com.oracle.graal.python.nodes.function.builtins.clinic.ArgumentClinicProvider;
5856
import com.oracle.graal.python.nodes.statement.AbstractImportNode;
59-
import com.oracle.graal.python.runtime.AsyncHandler;
6057
import com.oracle.graal.python.runtime.ExecutionContext.IndirectCallContext;
6158
import com.oracle.graal.python.runtime.PythonContext;
62-
import com.oracle.graal.python.runtime.PythonCore;
6359
import com.oracle.graal.python.runtime.PythonOptions;
6460
import com.oracle.graal.python.runtime.exception.ExceptionUtils;
6561
import com.oracle.graal.python.runtime.exception.PException;
66-
import com.oracle.truffle.api.CompilerDirectives;
6762
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
63+
import com.oracle.truffle.api.ThreadLocalAction;
6864
import com.oracle.truffle.api.dsl.GenerateNodeFactory;
6965
import com.oracle.truffle.api.dsl.NodeFactory;
7066
import com.oracle.truffle.api.dsl.Specialization;
7167
import com.oracle.truffle.api.frame.VirtualFrame;
72-
import com.oracle.truffle.api.object.DynamicObjectLibrary;
73-
import com.oracle.truffle.api.object.HiddenKey;
7468

7569
@CoreFunctions(defineModule = "faulthandler")
7670
public class FaulthandlerModuleBuiltins extends PythonBuiltins {
77-
private static final HiddenKey STACK_DUMP_REQUESTED = new HiddenKey("stackDumpRequested");
78-
private WeakHashMap<Thread, Object> dumpRequestedForThreads = new WeakHashMap<>();
79-
8071
@Override
8172
protected List<? extends NodeFactory<? extends PythonBuiltinBaseNode>> getNodeFactories() {
8273
return FaulthandlerModuleBuiltinsFactory.getFactories();
@@ -96,52 +87,17 @@ private static void dumpTraceback(Object callable, Object file) {
9687
}
9788
}
9889

99-
private static final class StackDumpAction implements AsyncHandler.AsyncAction {
100-
@Override
101-
public void execute(PythonContext context) {
102-
CompilerDirectives.bailout("This should never be compiled");
103-
PythonModule mod = context.getCore().lookupBuiltinModule("faulthandler");
104-
Object dumpQueue = DynamicObjectLibrary.getUncached().getOrDefault(mod, STACK_DUMP_REQUESTED, null);
105-
if (dumpQueue instanceof WeakHashMap) {
106-
WeakHashMap<?, ?> weakDumpQueue = (WeakHashMap<?, ?>) dumpQueue;
107-
Object callableAndFile = weakDumpQueue.remove(Thread.currentThread());
108-
if (callableAndFile instanceof Object[]) {
109-
Object callable = ((Object[]) callableAndFile)[0];
110-
Object file = ((Object[]) callableAndFile)[1];
111-
dumpTraceback(callable, file);
112-
}
113-
}
114-
}
115-
116-
private static final StackDumpAction INSTANCE = new StackDumpAction();
117-
}
118-
119-
@Override
120-
public void postInitialize(PythonCore core) {
121-
super.postInitialize(core);
122-
final PythonContext ctx = core.getContext();
123-
PythonModule mod = core.lookupBuiltinModule("faulthandler");
124-
mod.setAttribute(STACK_DUMP_REQUESTED, dumpRequestedForThreads);
125-
ctx.registerAsyncAction(() -> {
126-
if (!dumpRequestedForThreads.isEmpty()) {
127-
return StackDumpAction.INSTANCE;
128-
} else {
129-
return null;
130-
}
131-
});
132-
}
133-
134-
@Builtin(name = "dump_traceback", minNumOfPositionalArgs = 1, parameterNames = {"$mod", "file", "all_threads"}, declaresExplicitSelf = true)
90+
@Builtin(name = "dump_traceback", minNumOfPositionalArgs = 0, parameterNames = {"file", "all_threads"})
13591
@ArgumentClinic(name = "file", defaultValue = "PNone.NO_VALUE")
13692
@ArgumentClinic(name = "all_threads", conversion = ArgumentClinic.ClinicConversion.Boolean, defaultValue = "true")
13793
@GenerateNodeFactory
13894
abstract static class DumpTracebackNode extends PythonClinicBuiltinNode {
13995
@Specialization
140-
public PNone doit(VirtualFrame frame, PythonModule mod, Object file, boolean allThreads) {
96+
public PNone doit(VirtualFrame frame, Object file, boolean allThreads) {
14197
Object state = IndirectCallContext.enter(frame, getContext(), this);
14298
try {
14399
// it's not important for this to be fast at all
144-
dump(getContext(), mod, file, allThreads);
100+
dump(getContext(), file, allThreads);
145101
} finally {
146102
IndirectCallContext.exit(frame, getContext(), state);
147103
}
@@ -150,7 +106,7 @@ public PNone doit(VirtualFrame frame, PythonModule mod, Object file, boolean all
150106

151107
@TruffleBoundary
152108
@SuppressWarnings("unchecked")
153-
private static final void dump(PythonContext context, PythonModule mod, Object file, boolean allThreads) {
109+
private static final void dump(PythonContext context, Object file, boolean allThreads) {
154110
Object printStackFunc;
155111
try {
156112
Object tracebackModule = AbstractImportNode.importModule("traceback");
@@ -182,15 +138,12 @@ private static final void dump(PythonContext context, PythonModule mod, Object f
182138
}
183139
}
184140

185-
Object dumpQueue = DynamicObjectLibrary.getUncached().getOrDefault(mod, STACK_DUMP_REQUESTED, null);
186-
if (dumpQueue instanceof WeakHashMap) {
187-
WeakHashMap<Thread, Object[]> weakDumpQueue = (WeakHashMap<Thread, Object[]>) dumpQueue;
188-
if (weakDumpQueue.isEmpty()) {
189-
for (Thread th : context.getThreads()) {
190-
weakDumpQueue.put(th, new Object[]{printStackFunc, file});
191-
}
141+
context.getEnv().submitThreadLocal(null, new ThreadLocalAction(true, false) {
142+
@Override
143+
protected void perform(ThreadLocalAction.Access access) {
144+
dumpTraceback(printStackFunc, file);
192145
}
193-
}
146+
});
194147
} else {
195148
if (PythonOptions.isWithJavaStacktrace(context.getLanguage())) {
196149
PrintWriter err = new PrintWriter(context.getStandardErr());

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/GcModuleBuiltins.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,16 +68,22 @@ public void initialize(PythonCore core) {
6868
@GenerateNodeFactory
6969
abstract static class GcCollectNode extends PythonBuiltinNode {
7070
@Specialization
71+
@TruffleBoundary
7172
int collect(@SuppressWarnings("unused") Object level,
7273
@Cached GilNode gil) {
7374
gil.release(true);
7475
try {
7576
PythonUtils.forceFullGC();
77+
try {
78+
Thread.sleep(15);
79+
} catch (InterruptedException e) {
80+
// doesn't matter, just trying to give the GC more time
81+
}
7682
} finally {
7783
gil.acquire();
7884
}
7985
// collect some weak references now
80-
getContext().triggerAsyncActions();
86+
PythonContext.triggerAsyncActions(this);
8187
return 0;
8288
}
8389
}

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/PosixModuleBuiltins.java

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -410,8 +410,8 @@ private void execv(VirtualFrame frame, PosixPath path, Object argv, SequenceStor
410410

411411
auditNode.audit("os.exec", path.originalObject, argv, PNone.NONE);
412412

413+
gil.release(true);
413414
try {
414-
gil.release(true);
415415
posixLib.execv(getPosixSupport(), path.value, opaqueArgs);
416416
} catch (PosixException e) {
417417
gil.acquire();
@@ -498,11 +498,10 @@ int open(VirtualFrame frame, PosixPath path, int flags, int mode, int dirFd,
498498
return posixLib.openat(getPosixSupport(), dirFd, path.value, fixedFlags, mode);
499499
} catch (PosixException e) {
500500
errorProfile.enter();
501-
gil.acquire();
502501
if (e.getErrorCode() == OSErrorEnum.EINTR.getNumber()) {
503-
getContext().triggerAsyncActions();
504-
gil.release(true);
502+
PythonContext.triggerAsyncActions(this);
505503
} else {
504+
gil.acquire(); // need GIL to construct OSError
506505
throw raiseOSErrorFromPosixException(frame, e, path.originalObject);
507506
}
508507
}
@@ -580,9 +579,7 @@ public PBytes read(VirtualFrame frame, int fd, int length,
580579
} catch (PosixException e) {
581580
errorProfile.enter();
582581
if (e.getErrorCode() == OSErrorEnum.EINTR.getNumber()) {
583-
gil.acquire(); // need gil to trigger actions or construct OSError
584-
getContext().triggerAsyncActions();
585-
gil.release(true); // continue read loop without gil
582+
PythonContext.triggerAsyncActions(this);
586583
} else {
587584
throw e;
588585
}
@@ -629,9 +626,7 @@ public long write(int fd, byte[] data,
629626
} catch (PosixException e) {
630627
errorProfile.enter();
631628
if (e.getErrorCode() == OSErrorEnum.EINTR.getNumber()) {
632-
gil.acquire();
633-
getContext().triggerAsyncActions();
634-
gil.release(true);
629+
PythonContext.triggerAsyncActions(this);
635630
} else {
636631
throw e;
637632
}
@@ -824,7 +819,7 @@ PNone ftruncate(VirtualFrame frame, int fd, long length,
824819
} catch (PosixException e) {
825820
errorProfile.enter();
826821
if (e.getErrorCode() == OSErrorEnum.EINTR.getNumber()) {
827-
getContext().triggerAsyncActions();
822+
PythonContext.triggerAsyncActions(this);
828823
} else {
829824
throw raiseOSErrorFromPosixException(frame, e);
830825
}
@@ -854,7 +849,7 @@ PNone fsync(VirtualFrame frame, int fd,
854849
} catch (PosixException e) {
855850
errorProfile.enter();
856851
if (e.getErrorCode() == OSErrorEnum.EINTR.getNumber()) {
857-
getContext().triggerAsyncActions();
852+
PythonContext.triggerAsyncActions(this);
858853
} else {
859854
throw raiseOSErrorFromPosixException(frame, e);
860855
}
@@ -1030,7 +1025,7 @@ PTuple doStatFd(VirtualFrame frame, int fd,
10301025
} catch (PosixException e) {
10311026
errorProfile.enter();
10321027
if (e.getErrorCode() == OSErrorEnum.EINTR.getNumber()) {
1033-
getContext().triggerAsyncActions();
1028+
PythonContext.triggerAsyncActions(this);
10341029
} else {
10351030
throw raiseOSErrorFromPosixException(frame, e);
10361031
}
@@ -1252,7 +1247,7 @@ PNone fchdir(VirtualFrame frame, int fd,
12521247
} catch (PosixException e) {
12531248
errorProfile.enter();
12541249
if (e.getErrorCode() == OSErrorEnum.EINTR.getNumber()) {
1255-
getContext().triggerAsyncActions();
1250+
PythonContext.triggerAsyncActions(this);
12561251
} else {
12571252
throw raiseOSErrorFromPosixException(frame, e);
12581253
}
@@ -1772,6 +1767,7 @@ public abstract static class ExitNode extends PythonBuiltinNode {
17721767
@TruffleBoundary
17731768
@Specialization
17741769
Object exit(int status) {
1770+
// TODO: use a safepoint action to throw this exception to all running threads
17751771
throw new PythonExitException(this, status);
17761772
}
17771773
}
@@ -1792,21 +1788,23 @@ PTuple waitpid(VirtualFrame frame, long pid, int options,
17921788
@CachedLibrary("getPosixSupport()") PosixSupportLibrary posixLib,
17931789
@Cached BranchProfile errorProfile) {
17941790
gil.release(true);
1795-
while (true) {
1796-
try {
1797-
long[] result = posixLib.waitpid(getPosixSupport(), pid, options);
1798-
gil.acquire();
1799-
return factory().createTuple(new Object[]{result[0], result[1]});
1800-
} catch (PosixException e) {
1801-
errorProfile.enter();
1802-
gil.acquire();
1803-
if (e.getErrorCode() == OSErrorEnum.EINTR.getNumber()) {
1804-
getContext().triggerAsyncActions();
1805-
gil.release(true);
1806-
} else {
1807-
throw raiseOSErrorFromPosixException(frame, e);
1791+
try {
1792+
while (true) {
1793+
try {
1794+
long[] result = posixLib.waitpid(getPosixSupport(), pid, options);
1795+
return factory().createTuple(new Object[]{result[0], result[1]});
1796+
} catch (PosixException e) {
1797+
errorProfile.enter();
1798+
if (e.getErrorCode() == OSErrorEnum.EINTR.getNumber()) {
1799+
PythonContext.triggerAsyncActions(this);
1800+
} else {
1801+
gil.acquire();
1802+
throw raiseOSErrorFromPosixException(frame, e);
1803+
}
18081804
}
18091805
}
1806+
} finally {
1807+
gil.acquire();
18101808
}
18111809
}
18121810
}
@@ -1958,9 +1956,9 @@ int system(PBytes command,
19581956
// conversions for emulated backend because the bytes version after fsencode conversion
19591957
// is subject to sys.audit.
19601958
auditNode.audit("os.system", command);
1959+
byte[] bytes = toBytesNode.execute(command);
19611960
gil.release(true);
19621961
try {
1963-
byte[] bytes = toBytesNode.execute(command);
19641962
Object cmdOpaque = posixLib.createPathFromBytes(getPosixSupport(), bytes);
19651963
return posixLib.system(getPosixSupport(), cmdOpaque);
19661964
} finally {

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/TimeModuleBuiltins.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@
7575
import com.oracle.graal.python.nodes.util.CannotCastException;
7676
import com.oracle.graal.python.nodes.util.CastToJavaDoubleNode;
7777
import com.oracle.graal.python.runtime.GilNode;
78+
import com.oracle.graal.python.runtime.PythonContext;
7879
import com.oracle.graal.python.runtime.PythonCore;
7980
import com.oracle.graal.python.util.PythonUtils;
8081
import com.oracle.truffle.api.CompilerDirectives;
@@ -485,10 +486,10 @@ Object sleep(PythonModule self, long seconds,
485486
try {
486487
doSleep(seconds, deadline);
487488
} finally {
488-
dylib.put(self, TIME_SLEPT, nanoTime() - t + timeSlept(self));
489489
gil.acquire();
490+
dylib.put(self, TIME_SLEPT, nanoTime() - t + timeSlept(self));
490491
}
491-
getContext().triggerAsyncActions();
492+
PythonContext.triggerAsyncActions(this);
492493
return PNone.NONE;
493494
}
494495

@@ -511,7 +512,7 @@ Object sleep(PythonModule self, double seconds,
511512
gil.acquire();
512513
dylib.put(self, TIME_SLEPT, nanoTime() - t + timeSlept(self));
513514
}
514-
getContext().triggerAsyncActions();
515+
PythonContext.triggerAsyncActions(this);
515516
return PNone.NONE;
516517
}
517518

0 commit comments

Comments
 (0)