4 ms·

Not sure but there may be two bugs in the code. The use of notify on the condition outside of the mutex lock. I know this would cause problems with boost, not

Not sure but there may be two bugs in the code. The use of notify on the condition outside of the mutex lock. I know this would cause problems with boost, not sure about std::.

brandmeyer 7y ago
> Not sure but there may be two bugs in the code. The use of notify on the condition outside of the mutex lock. That's not a bug. The notifier can either signal the condition variable with or without the corresponding lock held and both cases are race-free. In some implementations, it may be more performant to signal the condvar with the lock still held, while in others it is more performant to signal the condvar after releasing the lock. In this discussion "implementations" isn't referring to boost, libstdc++, or LLVM's glue library, "implementations" is referring to the underlying system libraries. Alas, most of the implementations aren't willing to tell you which one is better.
- 0xffff2 7y ago
  I understand the case where it's more performant to notify outside of the lock, so that the notified thread doesn't wake up and immediately block again waiting for the notifying thread to release the lock. What would the case where it's more performant to notify while holding the lock look like? Is the underlying implementation somehow able to transfer ownership of the mutex?
  - brandmeyer 7y ago
    I must admit that I could be mistaken. There was a stack overflow question about this very topic wherein an NPTL implementer spoke up and claimed the following (any errors in this are the fault of my own recollection): > There is at least one implementation that takes advantage of the requirement that a condition variable be associated with a unique lock to make fewer futex(2) calls, such that the signaling and woken thread make exactly one system call each. NPTL does this. However, I cannot find that Q&A pairing any more. It is certainly the case on the RTOS I'm using right now that it is more optimal to signal outside the lock and rely on the fact that uncontended locking and unlocking operations don't make passes through the scheduler.
    - gpderetta 7y ago
      You are thinking of FUTEX_*_REQUEUE (see the man page for details). It will move (some of) the waiters from the condvar futex to the mutex futex. IIRC, the optimization is called wait morphing. IIRC it is so hard to get it right in practice (the number of races and corner cases is staggering) that recent libc versions might have stopped doing it. I might be misremembering though.
  - petters 7y ago
    I read somewhere that pthreads can transfer ownership.
thestoicattack 7y ago
Per [1] the lock does not need to be held when calling notify. In the 17 standard this is §33.5 but I can't quite parse out the real requirements there. [1] https://en.cppreference.com/w/cpp/thread/condition_variable https://en.cppreference.com/w/cpp/thread/condition_variable
- graetzer 7y ago
  Its usually a good idea to hold the lock because the other thread(s) might not have called wait() yet. As a rule of thumb: its usually a bug to not hold the lock before notify(), unless you synchronize it with another condition
  - usefulcat 7y ago
    In this case I think it should be ok because !this->tasks.empty() is part of the predicate, and will be true after a new item is enqueued (and before notify is called). So if the producer is interrupted between releasing the mutex and calling notify, the consumer would not call wait. But yeah, in general I agree with the advice.. I'm pretty sure I've made that mistake before.
    - ComputerGuru 7y ago
      It depends on if the predicate can (also) be changed without the lock being held, as otherwise the release/acquire ordering semantics are not guaranteed. So long as items are only every enqueued with the lock held, it should be ok.
      - gpderetta 7y ago
        according to cppreference: "Even if the shared variable is atomic, it must be modified under the mutex in order to correctly publish the modification to the waiting thread." That's not normative but I can't be bothered to check the c++ standard for exact wordings. Also SuSv4 doesn't have an explicit prohibition on modifying the predicate outside a critical section. But if you do, you are truly on your own.

Not sure but there may be two bugs in the code. The use of notify on the condition outside of the mutex lock. I know this would cause problems with boost, not sure about std::.

brandmeyer 7y ago

> Not sure but there may be two bugs in the code. The use of notify on the condition outside of the mutex lock. That's not a bug. The notifier can either signal the condition variable with or without the corresponding lock held and both cases are race-free. In some implementations, it may be more performant to signal the condvar with the lock still held, while in others it is more performant to signal the condvar after releasing the lock. In this discussion "implementations" isn't referring to boost, libstdc++, or LLVM's glue library, "implementations" is referring to the underlying system libraries. Alas, most of the implementations aren't willing to tell you which one is better.

0xffff2 7y ago
I understand the case where it's more performant to notify outside of the lock, so that the notified thread doesn't wake up and immediately block again waiting for the notifying thread to release the lock. What would the case where it's more performant to notify while holding the lock look like? Is the underlying implementation somehow able to transfer ownership of the mutex?
- brandmeyer 7y ago
  I must admit that I could be mistaken. There was a stack overflow question about this very topic wherein an NPTL implementer spoke up and claimed the following (any errors in this are the fault of my own recollection): > There is at least one implementation that takes advantage of the requirement that a condition variable be associated with a unique lock to make fewer futex(2) calls, such that the signaling and woken thread make exactly one system call each. NPTL does this. However, I cannot find that Q&A pairing any more. It is certainly the case on the RTOS I'm using right now that it is more optimal to signal outside the lock and rely on the fact that uncontended locking and unlocking operations don't make passes through the scheduler.
  - gpderetta 7y ago
    You are thinking of FUTEX_*_REQUEUE (see the man page for details). It will move (some of) the waiters from the condvar futex to the mutex futex. IIRC, the optimization is called wait morphing. IIRC it is so hard to get it right in practice (the number of races and corner cases is staggering) that recent libc versions might have stopped doing it. I might be misremembering though.
- petters 7y ago
  I read somewhere that pthreads can transfer ownership.

thestoicattack 7y ago

Per [1] the lock does not need to be held when calling notify. In the 17 standard this is §33.5 but I can't quite parse out the real requirements there. [1] https://en.cppreference.com/w/cpp/thread/condition_variable https://en.cppreference.com/w/cpp/thread/condition_variable

graetzer 7y ago
Its usually a good idea to hold the lock because the other thread(s) might not have called wait() yet. As a rule of thumb: its usually a bug to not hold the lock before notify(), unless you synchronize it with another condition
- usefulcat 7y ago
  In this case I think it should be ok because !this->tasks.empty() is part of the predicate, and will be true after a new item is enqueued (and before notify is called). So if the producer is interrupted between releasing the mutex and calling notify, the consumer would not call wait. But yeah, in general I agree with the advice.. I'm pretty sure I've made that mistake before.
  - ComputerGuru 7y ago
    It depends on if the predicate can (also) be changed without the lock being held, as otherwise the release/acquire ordering semantics are not guaranteed. So long as items are only every enqueued with the lock held, it should be ok.
    - gpderetta 7y ago
      according to cppreference: "Even if the shared variable is atomic, it must be modified under the mutex in order to correctly publish the modification to the waiting thread." That's not normative but I can't be bothered to check the c++ standard for exact wordings. Also SuSv4 doesn't have an explicit prohibition on modifying the predicate outside a critical section. But if you do, you are truly on your own.