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

Fix overflow in kqueue backend #115

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Corendos
Copy link
Contributor

Closes #111

@mitchellh I managed to write a test to avoid regressions but I'm not sure it's deterministic, should I still add it?

When there is more than 256 events, the `events` slice could overflow
when processing completions.
@Corendos
Copy link
Contributor Author

While discussing with @steeve and re-reading the issue, I think it's better to add more context. There are basically two ways we can solve that.

1. Cap the amount of completion executed per tick

This is simply breaking from the loop as I did in this PR.

One issue with that is that, if we are in loop.run(.once), we will potentially "miss" completions because we are capped to 256 completions in one tick. We will thus give back control to the user and the remaining completions will be handled in the next tick.

2. Drain all the completions in one tick

For that, we need to add another loop around the while (self.completions.pop()) |c| { ... } block, so that we handle all the completions that are ready.

One issue with this solution is that it can end up needing multiple call to kevent_syscall and it kind of feels weird with respect to the .once


So, in the end, that's why I went for the first solution, but I can get that it's not what you would expect @mitchellh. Let me know what you feel is best, and I'll make it work 😁

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.

[kqueue] More than 256 Kevents per tick causes OOB
1 participant