Asynchronously load and unload a queue This code is supposed to asynchronously load a queue from a generating function.

#include &lt;queue&gt;
#include &lt;functional&gt;
#include &lt;thread&gt;
#include &lt;mutex&gt;
#include &lt;condition_variable&gt;
#include &lt;iostream&gt;
#include &lt;stdexcept&gt;

using namespace std;

template &lt;typename Container&gt;
struct LoadAsync {

    LoadAsync( size_t queue_size, function&lt;Container()&gt; f )
        : find_next( f ), size( queue_size ),
          worker_thread( &amp;LoadAsync&lt;Container&gt;::worker_thread_function, this )
    {
        // first, build the basics
        unique_lock&lt;mutex&gt; lk( m );
        cv_init.wait( lk, [this] { return this-&gt;qued.size() &gt;= this-&gt;size; } );
    }

    LoadAsync( const LoadAsync&amp; ) = delete;

    void worker_thread_function()
    {
        while ( true ) {
            unique_lock&lt;mutex&gt; lk( m );
            cv.wait( lk, [this] { return this-&gt;qued.size() &lt; this-&gt;size; } );
            while ( this-&gt;qued.size() &lt; this-&gt;size ) {
                try {
                    qued.emplace( find_next() );
                } catch (domain_error) {
                    this-&gt;size--;
                }
            }
            lk.unlock();
            cv_init.notify_one();
        }
    }

    Container pop_front()
    {
        unique_lock&lt;mutex&gt; lk( m );
        cv_init.wait( lk, [this] { return this-&gt;qued.size() &gt;= this-&gt;size; } );
        if (this-&gt;qued.size() == 0)
            throw out_of_range("out of range");
        auto front = move( qued.front() );
        qued.pop();
        lk.unlock();
        cv.notify_one();
        return front;
    }

  private:
    size_t size;
    queue&lt;Container&gt; qued;
    function&lt;Container()&gt; find_next;
    thread worker_thread;
    mutex m;
    condition_variable cv;
    condition_variable cv_init;
};

int main()
{
    auto func = []() {
        static int which = 1;
        if ( which &lt; 10 )
            return which++;
        else
            throw domain_error("out of range");
    };

    LoadAsync&lt;int&gt; la( 3, func );

    for ( int i = 0; i &lt; 11; ++i ) cout &lt;&lt; la.pop_front() &lt;&lt; endl;

}


Is this a good approach?  Are there any gotchas here in using condition_variables?  Are there any long bottlenecks here?  Is waiting on a condition variable an immediate thing?
I see one instance of undefined behavior in your code.

LoadAsync( size_t queue_size, function&lt;Container()&gt; f )
        : find_next( f ), size( queue_size ),
          worker_thread( &amp;LoadAsync&lt;Container&gt;::worker_thread_function, this )
...

void worker_thread_function()
{
    while ( true ) {
        unique_lock&lt;mutex&gt; lk( m );

...

thread worker_thread;
mutex m;


The LoadAsync constructor constructs each member variable in the order in which they were declared — i.e., it constructs this-&gt;worker_thread before constructing this-&gt;m. And the way you construct worker_thread causes it to start running immediately, and the first thing it does is attempt to lock m — whose constructor may not have finished yet!

The simplest solution is to reorder your member variables so that worker_thread will be constructed last.



Style notes:


using namespace std; is a bad idea, and you shouldn't do it (especially in a header file like this). Explicitly qualify std::mutex, std::domain_error, and so on. It's a good habit to get into.
&amp;LoadAsync&lt;Container&gt;::worker_thread_function is a verbose way of saying &amp;LoadAsync::worker_thread_function: the &lt;Container&gt; part is implied in this context, just as it is in your copy-constructor declaration.
You don't need to explicitly = delete your copy-constructor: since one of your non-static member variables is a non-movable std::mutex, your class itself becomes non-movable by default.
cv and cv_init are bad names. Consider cv_consume and cv_produce, or cv_queue_empty and cv_queue_full. (Also, it's strange that your pop_front will block waiting for the producer to finish running find_next() even when the queue isn't empty. Doesn't that basically defeat the purpose of having two threads?)
qued is a weird name; perhaps you meant q or queue?
Container is a bad name for the object type being contained by your queue. I suggest renaming it T.




You don't provide any way to clean up after your worker_thread. By default, std::thread's destructor simply blocks until the thread finishes, which (since your worker thread is just an infinite loop) means that destroying a LoadAsync object will hang forever.

Calling worker_thread.detach() is one solution, but will still "leak resources" in a sense. It would be better to change the worker_thread's condition from while (true) to something like while (!this-&gt;finished), change your predicate from

[this] { return this-&gt;qued.size() &lt; this-&gt;size; }


to

[this] { return finished || (qued.size() &lt; size); }


and then change the body of the destructor to

{
    unique_lock&lt;mutex&gt; lk(m);
    this-&gt;finished = true;
}
this-&gt;cv.notify_one();
this-&gt;worker_thread.join();




It's bad practice to call the user-provided function find_next() while holding the lock on m. Consider what would happen if find_next tried to take the same lock:

bool first = true;
LoadAsync&lt;int&gt; la(3, [&amp;]() {
    return first ? (first = false, 42) : la.pop_front() + 1;
});


The easiest solution is to drop the lock, call find_next(), and then reacquire the lock to move the result into the queue. Of course, the move-constructor of Container might take the lock...

Also, notice that if find_next throws anything other than std::domain_error, your program will die in std::terminate. That's not so good.



Lastly, notice that you're relying on the notion of a "producer thread" and a "consumer thread" being able to run concurrently with each other. If your program is running in a context so resource-constrained that it can only get hold of a single thread, then your code will deadlock. IMHO that's extremely unlikely, and even if it did happen I would hope that thread's constructor would throw an exception instead of blithely continuing on its way. But I don't know.