Skip to content

Do not lock while calling signal handles #1948

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 24, 2025
Merged

Do not lock while calling signal handles #1948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 19 additions & 42 deletions src/core/IronPython.Modules/signal.NtSignalState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,51 +49,28 @@ private bool WindowsEventHandler(uint winSignal) {
pySignal = SIGBREAK;
break;
default:
throw new Exception("unreachable");
throw new InvalidOperationException("unreachable");
}

lock (PySignalToPyHandler) {
if (PySignalToPyHandler[pySignal].GetType() == typeof(int)) {
int tempId = (int)PySignalToPyHandler[pySignal];

if (tempId == SIG_DFL) {
// SIG_DFL - we let Windows do whatever it normally would
retVal = false;
} else if (tempId == SIG_IGN) {
// SIG_IGN - we do nothing, but tell Windows we handled the signal
retVal = true;
} else {
throw new Exception("unreachable");
}
} else if (PySignalToPyHandler[pySignal] == default_int_handler) {
if (pySignal != SIGINT) {
// We're dealing with the default_int_handlerImpl which we
// know doesn't care about the frame parameter
retVal = true;
default_int_handlerImpl(pySignal, null);
} else {
// Let the real interrupt handler throw a KeyboardInterrupt for SIGINT.
// It handles this far more gracefully than we can
retVal = false;
}
} else {
// We're dealing with a callable matching PySignalHandler's signature
object? handler = PySignalToPyHandler[pySignal];

if (handler is int tempId) {
if (tempId == SIG_DFL) {
// SIG_DFL - we let Windows do whatever it normally would
retVal = false;
} else if (tempId == SIG_IGN) {
// SIG_IGN - we do nothing, but tell Windows we handled the signal
retVal = true;
PySignalHandler temp = (PySignalHandler)Converter.ConvertToDelegate(PySignalToPyHandler[pySignal],
typeof(PySignalHandler));

try {
if (SignalPythonContext.PythonOptions.Frames) {
temp.Invoke(pySignal, SysModule._getframeImpl(null,
0,
SignalPythonContext._mainThreadFunctionStack));
} else {
temp.Invoke(pySignal, null);
}
} catch (Exception e) {
System.Console.WriteLine(SignalPythonContext.FormatException(e));
}
} else {
throw new InvalidOperationException("unreachable");
}
} else if (handler == default_int_handler && pySignal == SIGINT) {
// Let the real interrupt handler throw a KeyboardInterrupt for SIGINT.
// It handles this far more gracefully than we can
retVal = false;
} else {
// We're dealing with a callable matching PySignalHandler's signature
CallPythonHandler(pySignal, handler);
retVal = true;
}

return retVal;
Expand Down
62 changes: 18 additions & 44 deletions src/core/IronPython.Modules/signal.SimpleSignalState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ namespace IronPython.Modules {
public static partial class PythonSignal {
private class SimpleSignalState : PythonSignalState {

public SimpleSignalState(PythonContext pc)
: base(pc) {
public SimpleSignalState(PythonContext pc) : base(pc) {
Console.CancelKeyPress += new ConsoleCancelEventHandler(Console_CancelKeyPress);
}

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

lock (PySignalToPyHandler) {
if (PySignalToPyHandler[pySignal].GetType() == typeof(int)) {
int tempId = (int)PySignalToPyHandler[pySignal];

if (tempId == SIG_DFL) {
// SIG_DFL - do whatever it normally would
return;
} else if (tempId == SIG_IGN) {
// SIG_IGN - we do nothing, but tell the OS we handled the signal
e.Cancel = false;
return;
} else {
throw new Exception("unreachable");
}
} else if (PySignalToPyHandler[pySignal] == default_int_handler) {
if (pySignal != SIGINT) {
// We're dealing with the default_int_handlerImpl which we
// know doesn't care about the frame parameter
e.Cancel = true;
default_int_handlerImpl(pySignal, null);
return;
} else {
// Let the real interrupt handler throw a KeyboardInterrupt for SIGINT.
// It handles this far more gracefully than we can
return;
}
} else {
// We're dealing with a callable matching PySignalHandler's signature
PySignalHandler temp = (PySignalHandler)Converter.ConvertToDelegate(PySignalToPyHandler[pySignal],
typeof(PySignalHandler));

try {
if (SignalPythonContext.PythonOptions.Frames) {
temp.Invoke(pySignal, SysModule._getframeImpl(null,
0,
SignalPythonContext._mainThreadFunctionStack));
} else {
temp.Invoke(pySignal, null);
}
} catch (Exception ex) {
System.Console.WriteLine(SignalPythonContext.FormatException(ex));
}
object? handler = PySignalToPyHandler[pySignal];

if (handler is int tempId) {
if (tempId == SIG_DFL) {
// SIG_DFL - do whatever it normally would
return;
} else if (tempId == SIG_IGN) {
// SIG_IGN - we do nothing, but tell the OS we handled the signal
e.Cancel = true;
return;
} else {
throw new InvalidOperationException("unreachable");
}
} else if (ReferenceEquals(handler, default_int_handler) && pySignal == SIGINT) {
// Let the real interrupt handler throw a KeyboardInterrupt for SIGINT.
// It handles this far more gracefully than we can
return;
} else {
CallPythonHandler(pySignal, handler);
e.Cancel = true;
return;
}
}
}
Expand Down
107 changes: 85 additions & 22 deletions src/core/IronPython.Modules/signal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,16 +314,13 @@ public static object default_int_handlerImpl(int signalnum, TraceBackFrame? fram
anything else -- the callable Python object used as a handler
""")]
public static object? getsignal(CodeContext/*!*/ context, int signalnum) {
lock (GetPythonSignalState(context).PySignalToPyHandler) {
// Negative Scenarios
if (signalnum <= 0 || signalnum >= NSIG) {
throw PythonOps.ValueError("signal number out of range");
} else if (GetPythonSignalState(context).PySignalToPyHandler.TryGetValue(signalnum, out object? value)) {
// Default
if (signalnum <= 0 || signalnum >= NSIG) throw PythonOps.ValueError("signal number out of range");

var state = GetPythonSignalState(context);
lock (state.SyncRoot) {
if (state.TryGetPyHandler(signalnum, out object? value)) {
return value;
} else {
// Handles the special case of SIG_IGN. This is not really a signal,
// but CPython returns null for it any ways
return null;
}
}
Expand Down Expand Up @@ -375,12 +372,16 @@ public static object default_int_handlerImpl(int signalnum, TraceBackFrame? fram
if (signalnum == SIGKILL || signalnum == SIGSTOP) throw PythonNT.GetOsError(PythonErrno.EINVAL);
}
object? last_handler = null;
lock (GetPythonSignalState(context).PySignalToPyHandler) {
var state = GetPythonSignalState(context);
lock (state.SyncRoot) {
// CPython returns the previous handler for the signal
last_handler = getsignal(context, signalnum);
if (last_handler is null) throw PythonNT.GetOsError(PythonErrno.EINVAL);
if (!state.TryGetPyHandler(signalnum, out last_handler) || last_handler is null) {
// null marks signals that cannot be handled or are unsupported
throw PythonNT.GetOsError(PythonErrno.EINVAL);
}

// Set the new action
GetPythonSignalState(context).PySignalToPyHandler[signalnum] = action;
state.SetPyHandler(signalnum, action);
}

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


/// <summary>
/// This class is used to store the installed signal handlers.
/// </summary>
private class PythonSignalState {
// this provides us with access to the Main thread's stack
public PythonContext SignalPythonContext;

// Map out signal identifiers to their actual handlers
public Dictionary<int, object> PySignalToPyHandler;
public readonly PythonContext SignalPythonContext;

public object SyncRoot => PySignalToPyHandler;

/// <summary>
/// Map out signal identifiers to their actual handlers.
/// </summary>
/// <remarks>
/// The objects in this array are either:
/// 1. Int32(SIG_DFL) - let the OS handle the signal in the default way;
/// 2. Int32(SIG_IGN) - ignore the signal;
/// 3. a callable Python object - the handler for the signal;
/// 4. null - the signal (or handling thereof) is not supported on this platform.
/// </remarks>
protected readonly object?[] PySignalToPyHandler;

public PythonSignalState(PythonContext pc) {
SignalPythonContext = pc;
PySignalToPyHandler = new object[NSIG];

object sig_dfl = ScriptingRuntimeHelpers.Int32ToObject(SIG_DFL);
object sig_ign = ScriptingRuntimeHelpers.Int32ToObject(SIG_IGN);

int[] sigs = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? _PySupportedSignals_Windows
: RuntimeInformation.IsOSPlatform(OSPlatform.OSX) ? _PySupportedSignals_MacOS
: RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? _PySupportedSignals_Linux
: throw new NotSupportedException("Unsupported platform for signal module");

PySignalToPyHandler = new Dictionary<int, object>(sigs.Length);
object sig_dfl = ScriptingRuntimeHelpers.Int32ToObject(SIG_DFL);
object sig_ign = ScriptingRuntimeHelpers.Int32ToObject(SIG_IGN);
// Setting all defined signals to SIG_DFL
foreach (int sig in sigs) {
PySignalToPyHandler[sig] = sig_dfl;
}
if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) {
for (int sig = SIGRTMIN; sig <= SIGRTMAX; sig++) {
PySignalToPyHandler[sig] = sig_dfl;
}
}

// Setting exceptions to the rule
PySignalToPyHandler[SIGINT] = default_int_handler;
if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) {
PySignalToPyHandler[SIGPIPE] = sig_ign;
PySignalToPyHandler[SIGXFSZ] = sig_ign;
}
if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) {
PySignalToPyHandler.Remove(SIGKILL);
PySignalToPyHandler.Remove(SIGSTOP);
PySignalToPyHandler[SIGKILL] = null;
PySignalToPyHandler[SIGSTOP] = null;
}
}


public virtual bool TryGetPyHandler(int signalnum, out object? value)
=> (value = PySignalToPyHandler[signalnum]) != null;


public virtual void SetPyHandler(int signalnum, object value)
=> PySignalToPyHandler[signalnum] = value;


/// <summary>
/// Call the Python handler callable passing the given signal number as argument to the callable.
/// </summary>
protected void CallPythonHandler(int signum, object? handler) {
if (handler is null) return;

if (handler == default_int_handler) {
// We're dealing with the default_int_handlerImpl which we
// know doesn't care about the frame parameter
default_int_handlerImpl(signum, null);
return;
} else {
// We're dealing with a callable matching PySignalHandler's signature
try {
PySignalHandler temp = (PySignalHandler)Converter.ConvertToDelegate(handler,
typeof(PySignalHandler));

if (SignalPythonContext.PythonOptions.Frames) {
temp.Invoke(signum, SysModule._getframeImpl(null,
0,
SignalPythonContext._mainThreadFunctionStack));
} else {
temp.Invoke(signum, null);
}
} catch (Exception ex) {
System.Console.WriteLine(SignalPythonContext.FormatException(ex));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Console.WriteLine is a bit weird, but I guess that was there before... so LGTM!

Copy link
Member Author

@BCSharp BCSharp Apr 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is weird and yes, it was like that before. It seems that CPython prints it on stderr and then terminates. I will revisit it later once the signals on POSIX are working.

}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/suite/modules/system_related/test_signal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def a(b, c):
# test that unsupported signals raise OSError on trying to set a handler
for x in range(1, signal.NSIG):
if x in SUPPORTED_SIGNALS: continue
if is_linux and 35 <= x <= 64: continue # Real-time signals
if is_linux and signal.SIGRTMIN <= x <= signal.SIGRTMAX: continue # Real-time signals
self.assertRaisesMessage(ValueError if is_windows else OSError, "invalid signal value" if is_windows else "[Errno 22] Invalid argument",
signal.signal, x, a)

Expand Down