Skip to content
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.

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

Switch to crossbeam-queue for events #1091

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ahomescu
Copy link
Contributor

@ahomescu ahomescu commented May 6, 2024

Use a lock-free queue to store the event list because the event callbacks might be called from inside a signal handler. Most system calls are not safe in that context, so we just spin if the queue is full.

@ahomescu ahomescu self-assigned this May 6, 2024
Copy link
Contributor

@kkysen kkysen left a comment

Choose a reason for hiding this comment

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

Did we run into this causing problems? Signal safety is really annoying.

Use a lock-free queue to store the event list
because the event callbacks might be called from
inside a signal handler. Most system calls are not
safe in that context, so we just spin if the queue
is full.
@ahomescu ahomescu force-pushed the ahomescu/lock_free_event_queue branch from 64faf2f to d83f983 Compare May 9, 2024 04:59
@spernsteiner
Copy link
Contributor

What's left to do here?

Edit: Should I go audit all the functions in the runtime and check exactly which ones need to be signal-safe? Shouldn't be too hard.

@ahomescu, did you already do this / do you still plan to?

@ahomescu
Copy link
Contributor Author

ahomescu commented Jun 6, 2024

What's left to do here?

Edit: Should I go audit all the functions in the runtime and check exactly which ones need to be signal-safe? Shouldn't be too hard.

@ahomescu, did you already do this / do you still plan to?

I added a bunch of comments on signal safety, and removed a few potentially unsafe eprintln! calls.

analysis/runtime/src/handlers.rs Show resolved Hide resolved
analysis/runtime/src/runtime/scoped_runtime.rs Outdated Show resolved Hide resolved
analysis/runtime/src/runtime/skip.rs Show resolved Hide resolved
@ahomescu ahomescu force-pushed the ahomescu/lock_free_event_queue branch from c5eaa55 to a7fe739 Compare June 6, 2024 22:17
@ahomescu ahomescu force-pushed the ahomescu/lock_free_event_queue branch from a7fe739 to ca953b3 Compare June 6, 2024 22:21
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.

None yet

3 participants