Barrier implementation in C++ I've implemented a barreir in C++ close to one showed here. In the project code it's used only with two threads and seems to work. A synthetic test didn't expose any flaws too. But I suspect that on some machines it segfaults and/or falls through wait() call.
How do you think, can it fall through wait or is any race scenario possible with it?
Used mainly on Windows machines, if it matters.

class Barrier
{
public:
    explicit Barrier(uint16_t iCount) : maxThreads(iCount), count(iCount) {}

    void wait(std::chrono::milliseconds = std::chrono::milliseconds(0));

private:
    void notifyAll();

private:
    std::mutex mutex;
    std::condition_variable cv;
    uint16_t maxThreads;
    uint16_t count;
    bool generation = true;
};

inline void Barrier::wait(const std::chrono::milliseconds timeWait)
{
    std::unique_lock&lt;std::mutex&gt; ulock(mutex);
    auto gen = generation;
    const auto waitCondition = [this, gen]{ return (gen != generation); };
    if (0 == --count) notifyAll();
    else {
        if (timeWait == std::chrono::milliseconds(0)) cv.wait(ulock, waitCondition);
        else {
            const auto timedOut = !cv.wait_for(ulock, timeWait, waitCondition);
            if (timedOut) ++count;
        }
    }
}

inline void Barrier::notifyAll()
{
    generation = !generation;
    count = maxThreads;
    cv.notify_all();
}


P.S. I know that generation is poor naming for this particular variable. Here it's intended to express similarity to the same name variable from abovementioned SO question code.
Naming nitpicks: The relationship between count and maxThreads is not obvious. I would probably name them something like threads_arrived_ and total_threads_ (using the common convention of postfix-underscore for data members).

You might like to know that there's a std::barrier in C++20. Your wait is what C++20 calls arrive_and_wait.  Your generation is what C++20 calls the "phase."



Why is your notifyAll a separate function, given that it's private and called only in a single place? You should just inline it. As a bonus, you could then drop the mutex lock before calling cv.notify_all() — this is widely quoted as a performance optimization.



You wrote:

void wait(std::chrono::milliseconds = std::chrono::milliseconds(0));


Being as default arguments are the devil, this is a red flag. Personally I would write this as

void arrive_and_wait();
void arrive_and_wait_for(std::chrono::milliseconds timeout);


and continue adding an arrive_and_wait_until as well. I would not use overloading or default arguments to accomplish this, because "wait for 0 milliseconds" and "wait forever" are completely different things. You really really don't want someone doing, like,

barrier.wait(user_configured_timeout);      // wait for 1ms
barrier.wait(user_configured_timeout / 2);  // wait for half that


and getting an infinite wait instead. Follow the STL's lead here: wait, wait_for, and wait_until are all different operations.



if (0 == --count) notifyAll();
else {
    if (timeWait == std::chrono::milliseconds(0)) cv.wait(ulock, waitCondition);
    else {
        const auto timedOut = !cv.wait_for(ulock, timeWait, waitCondition);
        if (timedOut) ++count;
    }
}


This is a ridiculously misindented piece of code. Try this:

if (0 == --count) {
    notifyAll();
} else if (timeWait == std::chrono::milliseconds(0)) {
    // note that our refactored `wait_for` will not contain this branch
    cv.wait(ulock, waitCondition);
} else {
    // note that our refactored `wait` will not contain this branch
    bool timedOut = !cv.wait_for(ulock, timeWait, waitCondition);
    if (timedOut) ++count;
}


That ++count smells like a race condition to me. Sure, we're operating under the mutex lock here; but I suspect it's possible that some other thread might have observed the decremented value of count and acted on it, which might mean that here we're bumping count from 0 to 1, or from maxThreads to maxThreads+1. I'm not sure that this can happen, but I would look at this codepath very closely if I were you.



Consider what happens in code like

Thread A         Thread B         Thread C

Barrier b(2);
b.wait();        b.wait();
                 &lt;notifyAll&gt;
                 b.wait();        b.wait();
                                  &lt;notifyAll&gt;
                 &lt;awaken&gt;
&lt;awaken&gt;
&lt;resume waiting&gt;


If I understand correctly, when thread A finally gets scheduled and belatedly awakens from its cv.wait, it will observe gen == generation because generation has been toggled twice since it went to sleep. So it will remain blocked, even though it should have become unblocked as a result of thread B's first b.wait().

I'm not sure how to fix this, but it might involve thread B being forced to block, itself, until all the other threads in the current phase have indicated that they're unblocked. In fact I'm not sure that that would even fix the issue.



It's worth mentioning that it is physically possible for the programmer to destroy a Barrier while some other thread is still blocked waiting on it. Presumably you're okay with having this be undefined behavior.