Skip to content

Commit 74ce419

Browse files
BCSharpBCSharp
authored
Do not lock while calling signal handles (#1948)
1 parent 0c9866f commit 74ce419

File tree

4 files changed

+123
-109
lines changed

4 files changed

+123
-109
lines changed

‎src/core/IronPython.Modules/signal.NtSignalState.cs

Lines changed: 19 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -49,51 +49,28 @@ private bool WindowsEventHandler(uint winSignal) {
4949
pySignal = SIGBREAK;
5050
break;
5151
default:
52-
throw new Exception("unreachable");
52+
throw new InvalidOperationException("unreachable");
5353
}
54-
55-
lock (PySignalToPyHandler) {
56-
if (PySignalToPyHandler[pySignal].GetType() == typeof(int)) {
57-
int tempId = (int)PySignalToPyHandler[pySignal];
58-
59-
if (tempId == SIG_DFL) {
60-
// SIG_DFL - we let Windows do whatever it normally would
61-
retVal = false;
62-
} else if (tempId == SIG_IGN) {
63-
// SIG_IGN - we do nothing, but tell Windows we handled the signal
64-
retVal = true;
65-
} else {
66-
throw new Exception("unreachable");
67-
}
68-
} else if (PySignalToPyHandler[pySignal] == default_int_handler) {
69-
if (pySignal != SIGINT) {
70-
// We're dealing with the default_int_handlerImpl which we
71-
// know doesn't care about the frame parameter
72-
retVal = true;
73-
default_int_handlerImpl(pySignal, null);
74-
} else {
75-
// Let the real interrupt handler throw a KeyboardInterrupt for SIGINT.
76-
// It handles this far more gracefully than we can
77-
retVal = false;
78-
}
79-
} else {
80-
// We're dealing with a callable matching PySignalHandler's signature
54+
object? handler = PySignalToPyHandler[pySignal];
55+
56+
if (handler is int tempId) {
57+
if (tempId == SIG_DFL) {
58+
// SIG_DFL - we let Windows do whatever it normally would
59+
retVal = false;
60+
} else if (tempId == SIG_IGN) {
61+
// SIG_IGN - we do nothing, but tell Windows we handled the signal
8162
retVal = true;
82-
PySignalHandler temp = (PySignalHandler)Converter.ConvertToDelegate(PySignalToPyHandler[pySignal],
83-
typeof(PySignalHandler));
84-
85-
try {
86-
if (SignalPythonContext.PythonOptions.Frames) {
87-
temp.Invoke(pySignal, SysModule._getframeImpl(null,
88-
0,
89-
SignalPythonContext._mainThreadFunctionStack));
90-
} else {
91-
temp.Invoke(pySignal, null);
92-
}
93-
} catch (Exception e) {
94-
System.Console.WriteLine(SignalPythonContext.FormatException(e));
95-
}
63+
} else {
64+
throw new InvalidOperationException("unreachable");
9665
}
66+
} else if (handler == default_int_handler && pySignal == SIGINT) {
67+
// Let the real interrupt handler throw a KeyboardInterrupt for SIGINT.
68+
// It handles this far more gracefully than we can
69+
retVal = false;
70+
} else {
71+
// We're dealing with a callable matching PySignalHandler's signature
72+
CallPythonHandler(pySignal, handler);
73+
retVal = true;
9774
}
9875

9976
return retVal;

‎src/core/IronPython.Modules/signal.SimpleSignalState.cs

Lines changed: 18 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@ namespace IronPython.Modules {
1515
public static partial class PythonSignal {
1616
private class SimpleSignalState : PythonSignalState {
1717

18-
public SimpleSignalState(PythonContext pc)
19-
: base(pc) {
18+
public SimpleSignalState(PythonContext pc) : base(pc) {
2019
Console.CancelKeyPress += new ConsoleCancelEventHandler(Console_CancelKeyPress);
2120
}
2221

@@ -29,52 +28,27 @@ private void Console_CancelKeyPress(object? sender, ConsoleCancelEventArgs e) {
2928
_ => throw new InvalidOperationException("unreachable"),
3029
};
3130

32-
lock (PySignalToPyHandler) {
33-
if (PySignalToPyHandler[pySignal].GetType() == typeof(int)) {
34-
int tempId = (int)PySignalToPyHandler[pySignal];
35-
36-
if (tempId == SIG_DFL) {
37-
// SIG_DFL - do whatever it normally would
38-
return;
39-
} else if (tempId == SIG_IGN) {
40-
// SIG_IGN - we do nothing, but tell the OS we handled the signal
41-
e.Cancel = false;
42-
return;
43-
} else {
44-
throw new Exception("unreachable");
45-
}
46-
} else if (PySignalToPyHandler[pySignal] == default_int_handler) {
47-
if (pySignal != SIGINT) {
48-
// We're dealing with the default_int_handlerImpl which we
49-
// know doesn't care about the frame parameter
50-
e.Cancel = true;
51-
default_int_handlerImpl(pySignal, null);
52-
return;
53-
} else {
54-
// Let the real interrupt handler throw a KeyboardInterrupt for SIGINT.
55-
// It handles this far more gracefully than we can
56-
return;
57-
}
58-
} else {
59-
// We're dealing with a callable matching PySignalHandler's signature
60-
PySignalHandler temp = (PySignalHandler)Converter.ConvertToDelegate(PySignalToPyHandler[pySignal],
61-
typeof(PySignalHandler));
62-
63-
try {
64-
if (SignalPythonContext.PythonOptions.Frames) {
65-
temp.Invoke(pySignal, SysModule._getframeImpl(null,
66-
0,
67-
SignalPythonContext._mainThreadFunctionStack));
68-
} else {
69-
temp.Invoke(pySignal, null);
70-
}
71-
} catch (Exception ex) {
72-
System.Console.WriteLine(SignalPythonContext.FormatException(ex));
73-
}
31+
object? handler = PySignalToPyHandler[pySignal];
7432

33+
if (handler is int tempId) {
34+
if (tempId == SIG_DFL) {
35+
// SIG_DFL - do whatever it normally would
36+
return;
37+
} else if (tempId == SIG_IGN) {
38+
// SIG_IGN - we do nothing, but tell the OS we handled the signal
7539
e.Cancel = true;
7640
return;
41+
} else {
42+
throw new InvalidOperationException("unreachable");
7743
}
44+
} else if (ReferenceEquals(handler, default_int_handler) && pySignal == SIGINT) {
45+
// Let the real interrupt handler throw a KeyboardInterrupt for SIGINT.
46+
// It handles this far more gracefully than we can
47+
return;
48+
} else {
49+
CallPythonHandler(pySignal, handler);
50+
e.Cancel = true;
51+
return;
7852
}
7953
}
8054
}

‎src/core/IronPython.Modules/signal.cs

Lines changed: 85 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -314,16 +314,13 @@ public static object default_int_handlerImpl(int signalnum, TraceBackFrame? fram
314314
anything else -- the callable Python object used as a handler
315315
""")]
316316
public static object? getsignal(CodeContext/*!*/ context, int signalnum) {
317-
lock (GetPythonSignalState(context).PySignalToPyHandler) {
318-
// Negative Scenarios
319-
if (signalnum <= 0 || signalnum >= NSIG) {
320-
throw PythonOps.ValueError("signal number out of range");
321-
} else if (GetPythonSignalState(context).PySignalToPyHandler.TryGetValue(signalnum, out object? value)) {
322-
// Default
317+
if (signalnum <= 0 || signalnum >= NSIG) throw PythonOps.ValueError("signal number out of range");
318+
319+
var state = GetPythonSignalState(context);
320+
lock (state.SyncRoot) {
321+
if (state.TryGetPyHandler(signalnum, out object? value)) {
323322
return value;
324323
} else {
325-
// Handles the special case of SIG_IGN. This is not really a signal,
326-
// but CPython returns null for it any ways
327324
return null;
328325
}
329326
}
@@ -375,12 +372,16 @@ public static object default_int_handlerImpl(int signalnum, TraceBackFrame? fram
375372
if (signalnum == SIGKILL || signalnum == SIGSTOP) throw PythonNT.GetOsError(PythonErrno.EINVAL);
376373
}
377374
object? last_handler = null;
378-
lock (GetPythonSignalState(context).PySignalToPyHandler) {
375+
var state = GetPythonSignalState(context);
376+
lock (state.SyncRoot) {
379377
// CPython returns the previous handler for the signal
380-
last_handler = getsignal(context, signalnum);
381-
if (last_handler is null) throw PythonNT.GetOsError(PythonErrno.EINVAL);
378+
if (!state.TryGetPyHandler(signalnum, out last_handler) || last_handler is null) {
379+
// null marks signals that cannot be handled or are unsupported
380+
throw PythonNT.GetOsError(PythonErrno.EINVAL);
381+
}
382+
382383
// Set the new action
383-
GetPythonSignalState(context).PySignalToPyHandler[signalnum] = action;
384+
state.SetPyHandler(signalnum, action);
384385
}
385386

386387
return last_handler;
@@ -417,35 +418,97 @@ private static void SetPythonSignalState(CodeContext/*!*/ context, PythonSignalS
417418
context.LanguageContext.SetModuleState(_PythonSignalStateKey, pss);
418419
}
419420

420-
421+
/// <summary>
422+
/// This class is used to store the installed signal handlers.
423+
/// </summary>
421424
private class PythonSignalState {
422425
// this provides us with access to the Main thread's stack
423-
public PythonContext SignalPythonContext;
424-
425-
// Map out signal identifiers to their actual handlers
426-
public Dictionary<int, object> PySignalToPyHandler;
426+
public readonly PythonContext SignalPythonContext;
427+
428+
public object SyncRoot => PySignalToPyHandler;
429+
430+
/// <summary>
431+
/// Map out signal identifiers to their actual handlers.
432+
/// </summary>
433+
/// <remarks>
434+
/// The objects in this array are either:
435+
/// 1. Int32(SIG_DFL) - let the OS handle the signal in the default way;
436+
/// 2. Int32(SIG_IGN) - ignore the signal;
437+
/// 3. a callable Python object - the handler for the signal;
438+
/// 4. null - the signal (or handling thereof) is not supported on this platform.
439+
/// </remarks>
440+
protected readonly object?[] PySignalToPyHandler;
427441

428442
public PythonSignalState(PythonContext pc) {
429443
SignalPythonContext = pc;
444+
PySignalToPyHandler = new object[NSIG];
445+
446+
object sig_dfl = ScriptingRuntimeHelpers.Int32ToObject(SIG_DFL);
447+
object sig_ign = ScriptingRuntimeHelpers.Int32ToObject(SIG_IGN);
448+
430449
int[] sigs = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? _PySupportedSignals_Windows
431450
: RuntimeInformation.IsOSPlatform(OSPlatform.OSX) ? _PySupportedSignals_MacOS
432451
: RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? _PySupportedSignals_Linux
433452
: throw new NotSupportedException("Unsupported platform for signal module");
434453

435-
PySignalToPyHandler = new Dictionary<int, object>(sigs.Length);
436-
object sig_dfl = ScriptingRuntimeHelpers.Int32ToObject(SIG_DFL);
437-
object sig_ign = ScriptingRuntimeHelpers.Int32ToObject(SIG_IGN);
454+
// Setting all defined signals to SIG_DFL
438455
foreach (int sig in sigs) {
439456
PySignalToPyHandler[sig] = sig_dfl;
440457
}
458+
if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) {
459+
for (int sig = SIGRTMIN; sig <= SIGRTMAX; sig++) {
460+
PySignalToPyHandler[sig] = sig_dfl;
461+
}
462+
}
463+
464+
// Setting exceptions to the rule
441465
PySignalToPyHandler[SIGINT] = default_int_handler;
442466
if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) {
443467
PySignalToPyHandler[SIGPIPE] = sig_ign;
444468
PySignalToPyHandler[SIGXFSZ] = sig_ign;
445469
}
446470
if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) {
447-
PySignalToPyHandler.Remove(SIGKILL);
448-
PySignalToPyHandler.Remove(SIGSTOP);
471+
PySignalToPyHandler[SIGKILL] = null;
472+
PySignalToPyHandler[SIGSTOP] = null;
473+
}
474+
}
475+
476+
477+
public virtual bool TryGetPyHandler(int signalnum, out object? value)
478+
=> (value = PySignalToPyHandler[signalnum]) != null;
479+
480+
481+
public virtual void SetPyHandler(int signalnum, object value)
482+
=> PySignalToPyHandler[signalnum] = value;
483+
484+
485+
/// <summary>
486+
/// Call the Python handler callable passing the given signal number as argument to the callable.
487+
/// </summary>
488+
protected void CallPythonHandler(int signum, object? handler) {
489+
if (handler is null) return;
490+
491+
if (handler == default_int_handler) {
492+
// We're dealing with the default_int_handlerImpl which we
493+
// know doesn't care about the frame parameter
494+
default_int_handlerImpl(signum, null);
495+
return;
496+
} else {
497+
// We're dealing with a callable matching PySignalHandler's signature
498+
try {
499+
PySignalHandler temp = (PySignalHandler)Converter.ConvertToDelegate(handler,
500+
typeof(PySignalHandler));
501+
502+
if (SignalPythonContext.PythonOptions.Frames) {
503+
temp.Invoke(signum, SysModule._getframeImpl(null,
504+
0,
505+
SignalPythonContext._mainThreadFunctionStack));
506+
} else {
507+
temp.Invoke(signum, null);
508+
}
509+
} catch (Exception ex) {
510+
System.Console.WriteLine(SignalPythonContext.FormatException(ex));
511+
}
449512
}
450513
}
451514
}

‎tests/suite/modules/system_related/test_signal.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ def a(b, c):
8686
# test that unsupported signals raise OSError on trying to set a handler
8787
for x in range(1, signal.NSIG):
8888
if x in SUPPORTED_SIGNALS: continue
89-
if is_linux and 35 <= x <= 64: continue # Real-time signals
89+
if is_linux and signal.SIGRTMIN <= x <= signal.SIGRTMAX: continue # Real-time signals
9090
self.assertRaisesMessage(ValueError if is_windows else OSError, "invalid signal value" if is_windows else "[Errno 22] Invalid argument",
9191
signal.signal, x, a)
9292

0 commit comments

Comments
 (0)