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

Invalid overflow handling in CancelWait of the notifier #518

Open
pbrunet opened this issue Oct 3, 2023 · 4 comments
Open

Invalid overflow handling in CancelWait of the notifier #518

pbrunet opened this issue Oct 3, 2023 · 4 comments
Labels
good first issue Good for newcomers

Comments

@pbrunet
Copy link

pbrunet commented Oct 3, 2023

Describe the bug
When using the notifier, deadlock may happen when a cancel is called with the max value of epoch.

To Reproduce
I don't have repro step. I guess it is a timing issue :-(

Desktop (please complete the following information):

  • OS: Windows

Additional context
When setting the new state, we inc epoch and decr waiter but with a single waiter and epoch at its max value and no "mask", new state will be :
epoch = 0
waiter = 0
mask = 0
while mask should stay the same. Certainly something like this:

uint64_t newstate = state - kWaiterInc + kEpochInc;
newstate = static_cast<uint64_t>( ( newstate & ~kStackMask ) | static_cast<uint64_t>( state & kStackMask ) );

The same issue looks to be present when notify is called while there is a waiter in prepare_wait

@tsung-wei-huang tsung-wei-huang added the good first issue Good for newcomers label Oct 4, 2023
@kaushal-malpure
Copy link

Hey @tsung-wei-huang ,
I want to contribute to this issue, can you guide me for this? Thanks in advance.

@tsung-wei-huang
Copy link
Member

@kaushal-malpure this in fact is under the file taskflow/core/notifier.hpp - it's a standalone hpp file to implement worker notification.

@kaushal-malpure
Copy link

@tsung-wei-huang
Attempted to address the problem.
PR id - #560

@kaushal-malpure
Copy link

@tsung-wei-huang Can you please review the changes in the above pull request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants