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

kernel/queue: Re-introduce race workaround #25908

Closed
wants to merge 1 commit into from

Conversation

andyross
Copy link
Contributor

@andyross andyross commented Jun 2, 2020

The timeout changes accidentally reverted the workaround introduced in
commit b173e43 ("kernel/queue: Fix spurious NULL exit condition
when using timeouts"): because k_poll() does not atomically release a
lock, the usage in k_queue is inherently racy: a queue element can be
removed from a queue after k_poll() has returned but before the thread
gets to the list. This results in an early return with a NULL value.

Reintroduce the looping around k_poll(), updated to the newer timeout
API.

Signed-off-by: Andy Ross andrew.j.ross@intel.com

The timeout changes accidentally reverted the workaround introduced in
commit b173e43 ("kernel/queue: Fix spurious NULL exit condition
when using timeouts"): because k_poll() does not atomically release a
lock, the usage in k_queue is inherently racy: a queue element can be
removed from a queue after k_poll() has returned but before the thread
gets to the list.  This results in an early return with a NULL value.

Reintroduce the looping around k_poll(), updated to the newer timeout
API.

Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
@andyross andyross requested a review from andrewboie as a code owner June 2, 2020 17:23
@andyross
Copy link
Contributor Author

andyross commented Jun 2, 2020

Alternative fix for #25904 that some folks might find more conservative.

FWIW: I think this code is actually harder, more subtle, and more difficult to validate. I'd strongly recommend merging #25906 instead.

@carlescufi
Copy link
Member

carlescufi commented Jun 3, 2020

@andyross closing this since #25906 is merged now.

@carlescufi carlescufi closed this Jun 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants