concurrent-deque icon indicating copy to clipboard operation
concurrent-deque copied to clipboard

Possible error

Open ConorWilliams opened this issue 4 years ago • 2 comments

Hi there, on line 206 in steal() you have moved the ->get() call into the CAS if, (as oppose to before like Figure 1. in the reference material). I believe this introduces a bug as:

  1. Thread A executes a steal() and hangs just after successfully completing the CAS.
  2. A push occurs, from thread B, that overwrites the stolen item (due to buffer wrap around).
  3. A wakes up and retrieves the wrong item.

This could be fixed by moving the ->get() before the CAS branch but then a copy must be made for every steal, even the unsuccessful ones :(

ConorWilliams avatar Mar 11 '21 14:03 ConorWilliams

Good catch, thanks for taking the time to review and submit a bug report. I read your post on the C++ subreddit about your own implementation as well; good stuff.

ssbl avatar Mar 13 '21 12:03 ssbl

Additionaly, I think buffer wrap-around means that ->get() is required to be atomic, otherwise you could read a partially written value which would cause undefined behaviour upon destruction.

ConorWilliams avatar Mar 14 '21 21:03 ConorWilliams