Skip to content

Fix a race condition in pthreads call proxying waiting #12243

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from

Conversation

kripken
Copy link
Member

@kripken kripken commented Sep 17, 2020

I am not completely happy with this solution, see details in
the comment. ideas welcome!

@kripken kripken requested a review from juj September 17, 2020 01:12
@kripken kripken changed the title Fix a race condition in pthreads call proxying Fix a race condition in pthreads call proxying waiting Sep 17, 2020
kripken added a commit that referenced this pull request Sep 17, 2020
Comment on lines +422 to +424
// while also avoids a possible race condition, as call->operationDone
// can be set *after* we checked it but *before* we do the futex_wait. The
// futex_wait is not aware of call->operationDone which means if we wait
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused. Isn't the whole point of futex_wait to do an atomic compare-and-block operation? Why wouldn't it be aware of call->operationDone?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I think you're right, while this fixes the issue the diagnosis does look wrong.

@kripken
Copy link
Member Author

kripken commented Sep 22, 2020

I have found the actual cause here, and will open a refactoring PR and then a fix PR shortly.

@kripken kripken closed this Sep 22, 2020
@kripken kripken deleted the pthread3 branch September 22, 2020 23:26
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants