Skip to content

Conversation

@awni
Copy link
Member

@awni awni commented Mar 19, 2025

CC @davidkoski wdyt?


void interrupt_eval() {
interrupt_flag() = true;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need a way to clear this if you raced vs the eval. E.g. you didn't hit line 259 above it will still be in true state on the next call to eval()

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes good point. There is some subtlety here. I changed it to only set the interrupt flag if an eval is actually ongoing. If it's not ongoing then the flag is not set.

So if you call interrupt before you call eval on the other thread then the eval will keep going. I think this is kind of expected since there is nothing to interrupt and you don't want to interrupt future evals. But it does introduce the possibility of a race. I added a return value to interrupt_eval so the caller can check if the eval they were expecting to interrupt was actually interrupted.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems reasonable


if (interrupt_flag()) {
interrupt_flag() = false;
synchronizer.attach_event(Event{stream});
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this leave the current work enqueued and cause the eval to return? Or does it drain it (or is that not a thing)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Work that has already been enqueued will remain so. That includes CPU and GPU work. The GPU command buffers are all committed when this returns.

From https://developer.apple.com/documentation/metal/preparing-your-metal-app-to-run-in-the-background?language=objc we may have to wait for the command buffers to be scheduled.

@aPaleBlueDot
Copy link

aPaleBlueDot commented Jul 14, 2025

👀 I'd greatly appreciate this feature, as it'll solve the app crashes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants