Skip to content

FIX: macos: Use standard NSApp run loop in our input hook #28981

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 4 commits into from
Oct 30, 2024
+56 −54
Merged

FIX: macos: Use standard NSApp run loop in our input hook #28981

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b1822cf
FIX: macos: remove our custom PyOS_InputHook after our runloop stops
greglucas Oct 16, 2024
18909c7
Rework stdin handling on macos objective C
ksunden Oct 18, 2024
0e8baa5
Short circuit input hook when no windows are open
ksunden Oct 29, 2024
b2eaad1
Review comments
ksunden Oct 30, 2024
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
110 changes: 56 additions & 54 deletions src/_macosx.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,60 +40,84 @@
static bool keyChangeCapsLock = false;
/* Keep track of the current mouse up/down state for open/closed cursor hand */
static bool leftMouseGrabbing = false;
/* Keep track of whether stdin has been received */
static bool stdin_received = false;
static bool stdin_sigint = false;
// Global variable to store the original SIGINT handler
static PyOS_sighandler_t originalSigintAction = NULL;

// Signal handler for SIGINT, only sets a flag to exit the run loop
// Stop the current app's run loop, sending an event to ensure it actually stops
static void stopWithEvent() {
[NSApp stop: nil];
// Post an event to trigger the actual stopping.
[NSApp postEvent: [NSEvent otherEventWithType: NSEventTypeApplicationDefined
location: NSZeroPoint
modifierFlags: 0
timestamp: 0
windowNumber: 0
context: nil
subtype: 0
data1: 0
data2: 0]
atStart: YES];
}

// Signal handler for SIGINT, only argument matching for stopWithEvent
static void handleSigint(int signal) {
stdin_sigint = true;
stopWithEvent();
}

// Helper function to flush all events.
// This is needed in some instances to ensure e.g. that windows are properly closed.
// It is used in the input hook as well as wrapped in a version callable from Python.
static void flushEvents() {
while (true) {
NSEvent* event = [NSApp nextEventMatchingMask: NSEventMaskAny
untilDate: [NSDate distantPast]
inMode: NSDefaultRunLoopMode
dequeue: YES];
if (!event) {
break;
}
[NSApp sendEvent:event];
}
}

static int wait_for_stdin() {
@autoreleasepool {
stdin_received = false;
stdin_sigint = false;
// Short circuit if no windows are active
// Rely on Python's input handling to manage CPU usage
// This queries the NSApp, rather than using our FigureWindowCount because that is decremented when events still
// need to be processed to properly close the windows.
Copy link
Member

Choose a reason for hiding this comment

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

This will also make us play nicer with any other OSX apps you launch from Python (which I am not aware of any others....)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we use this method in the close() method too instead of keeping track of it ourselves?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are some folks embedding this in C++/objc it would appear: #27147

Copy link
Member

Choose a reason for hiding this comment

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

That is its own bit of fun, but I think this will not intersect with that issue because the only place the PyOS_InputHook is run is from the Python repl (either readline or the new one) so if they are embedding Python in another application (which means they make an interpreter object and then stuff strings into it to run Python code) so I expect the repl will never run.

Copy link
Member

Choose a reason for hiding this comment

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

oooh, just after I clicked comment I connected some more dots.

Yes, we probably should be using something like this in close and than may fix #27147 .

if (![[NSApp windows] count]) {
flushEvents();
return 1;
}

@autoreleasepool {
// Set up a SIGINT handler to interrupt the event loop if ctrl+c comes in too
originalSigintAction = PyOS_setsig(SIGINT, handleSigint);

// Create an NSFileHandle for standard input
NSFileHandle *stdinHandle = [NSFileHandle fileHandleWithStandardInput];


// Register for data available notifications on standard input
[[NSNotificationCenter defaultCenter] addObserverForName: NSFileHandleDataAvailableNotification
object: stdinHandle
queue: [NSOperationQueue mainQueue] // Use the main queue
usingBlock: ^(NSNotification *notification) {
// Mark that input has been received
stdin_received = true;
}
id notificationID = [[NSNotificationCenter defaultCenter] addObserverForName: NSFileHandleDataAvailableNotification
object: stdinHandle
queue: [NSOperationQueue mainQueue] // Use the main queue
usingBlock: ^(NSNotification *notification) {stopWithEvent();}
];

// Wait in the background for anything that happens to stdin
[stdinHandle waitForDataInBackgroundAndNotify];

// continuously run an event loop until the stdin_received flag is set to exit
while (!stdin_received && !stdin_sigint) {
// This loop is similar to the main event loop and flush_events which have
// Py_[BEGIN|END]_ALLOW_THREADS surrounding the loop.
// This should not be necessary here because PyOS_InputHook releases the GIL for us.
while (true) {
NSEvent *event = [NSApp nextEventMatchingMask: NSEventMaskAny
untilDate: [NSDate distantPast]
inMode: NSDefaultRunLoopMode
dequeue: YES];
if (!event) { break; }
[NSApp sendEvent: event];
}
}
// Run the application's event loop, which will be interrupted on stdin or SIGINT
[NSApp run];

// Remove the input handler as an observer
[[NSNotificationCenter defaultCenter] removeObserver: stdinHandle];
[[NSNotificationCenter defaultCenter] removeObserver: notificationID];


// Restore the original SIGINT handler upon exiting the function
PyOS_setsig(SIGINT, originalSigintAction);

return 1;
}
}
Expand Down Expand Up @@ -236,18 +260,7 @@ static void lazy_init(void) {
static PyObject*
stop(PyObject* self)
{
[NSApp stop: nil];
// Post an event to trigger the actual stopping.
[NSApp postEvent: [NSEvent otherEventWithType: NSEventTypeApplicationDefined
location: NSZeroPoint
modifierFlags: 0
timestamp: 0
windowNumber: 0
context: nil
subtype: 0
data1: 0
data2: 0]
atStart: YES];
stopWithEvent();
Py_RETURN_NONE;
}

Expand Down Expand Up @@ -382,20 +395,9 @@ static CGFloat _get_device_scale(CGContextRef cr)
// We run the app, matching any events that are waiting in the queue
// to process, breaking out of the loop when no events remain and
// displaying the canvas if needed.
NSEvent *event;

Py_BEGIN_ALLOW_THREADS

while (true) {
event = [NSApp nextEventMatchingMask: NSEventMaskAny
untilDate: [NSDate distantPast]
inMode: NSDefaultRunLoopMode
dequeue: YES];
if (!event) {
break;
}
[NSApp sendEvent:event];
}
flushEvents();

Py_END_ALLOW_THREADS

Expand Down
Loading