Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c++ > #85917 > unrolled thread

Move constructor in lock_guard safe?

Back to article view | Back to comp.lang.c++

Contents

  Move constructor in lock_guard safe? Marcel Mueller <news.5.maazl@spamgourmet.org> - 2022-08-14 11:50 +0200
    Re: Move constructor in lock_guard safe? Paavo Helde <eesnimi@osa.pri.ee> - 2022-08-14 15:01 +0300
      Re: Move constructor in lock_guard safe? Öö Tiib <ootiib@hot.ee> - 2022-08-14 06:01 -0700
      Re: Move constructor in lock_guard safe? "Chris M. Thomasson" <chris.m.thomasson.1@gmail.com> - 2022-08-14 12:00 -0700
        Re: Move constructor in lock_guard safe? Marcel Mueller <news.5.maazl@spamgourmet.org> - 2022-08-15 00:08 +0200
          Re: Move constructor in lock_guard safe? "Chris M. Thomasson" <chris.m.thomasson.1@gmail.com> - 2022-08-14 16:22 -0700
        Re: Move constructor in lock_guard safe? Paavo Helde <eesnimi@osa.pri.ee> - 2022-08-15 09:38 +0300
          Re: Move constructor in lock_guard safe? "Chris M. Thomasson" <chris.m.thomasson.1@gmail.com> - 2022-08-14 23:56 -0700
        Re: Move constructor in lock_guard safe? scott@slp53.sl.home (Scott Lurndal) - 2022-08-15 15:31 +0000
          Re: Move constructor in lock_guard safe? "Chris M. Thomasson" <chris.m.thomasson.1@gmail.com> - 2022-08-15 11:11 -0700

#85917 — Move constructor in lock_guard safe?

In following code fragment a thread object is detached from a class 
instance. (last line)

class X
{
   std::mutex Lock;
   std::thread Worker;
   std::condition_variable CV;

   // ...

   std::thread Dispose()
   { std::lock_guard<mutex> lock(Lock);
     State = DIE;
     CV.notify_one();
     return std::move(Worker);  // <-- this line
   }
};

The worker thread will terminate asynchronously and call the class 
destructor as last action. But it is essential the the Worker variable 
is no longer joinable at this point.

Is it guaranteed that the move constructor has been completed its work 
before the lock is released? Or may RVO or whatever prevent this?


Marcel

[toc] | [next] | [standalone]

#85920

14.08.2022 12:50 Marcel Mueller kirjutas:
> In following code fragment a thread object is detached from a class 
> instance. (last line)
> 
> class X
> {
>    std::mutex Lock;
>    std::thread Worker;
>    std::condition_variable CV;

These two are in the wrong order. The thread member must be the last 
one, otherwise it may start running and access CV before CV is even created.

> 
>    // ...
> 
>    std::thread Dispose()
>    { std::lock_guard<mutex> lock(Lock);
>      State = DIE;
>      CV.notify_one();
>      return std::move(Worker);  // <-- this line
>    }
> };
> 
> The worker thread will terminate asynchronously and call the class 
> destructor as last action. But it is essential the the Worker variable 
> is no longer joinable at this point.
> 
> Is it guaranteed that the move constructor has been completed its work 
> before the lock is released? Or may RVO or whatever prevent this?

AFAIK, all side effects from a return statement are guaranteed to be 
complete before local scope variables, incl. the lock guards, are 
destroyed. So this line by itself is technically fine.

[toc] | [prev] | [next] | [standalone]

#85921

On Sunday, 14 August 2022 at 15:02:03 UTC+3, Paavo Helde wrote:
> 14.08.2022 12:50 Marcel Mueller kirjutas: 
> > In following code fragment a thread object is detached from a class 
> > instance. (last line) 
> > 
> > class X 
> > { 
> >   std::mutex Lock; 
> >   std::thread Worker; 
> >   std::condition_variable CV;
> 
> These two are in the wrong order. The thread member must be the last 
> one, otherwise it may start running and access CV before CV is even created.

You may be guessed correctly, as from pseudo-code it is hard to tell
but verbatim there are no such issue as std::thread is 
default-constructed in "not owning a thread" state. It is kind of 
envelope class.

[toc] | [prev] | [next] | [standalone]

#85923

On 8/14/2022 5:01 AM, Paavo Helde wrote:
> 14.08.2022 12:50 Marcel Mueller kirjutas:
>> In following code fragment a thread object is detached from a class 
>> instance. (last line)
>>
>> class X
>> {
>>    std::mutex Lock;
>>    std::thread Worker;
>>    std::condition_variable CV;
> 
> These two are in the wrong order. The thread member must be the last 
> one, otherwise it may start running and access CV before CV is even 
> created.
[...]

Please, try not to create threads in an objects ctor when the thread is 
going to start working on the state of the object as soon as its up and 
running. I have had to debug full blown nightmares from other peoples 
code using that horror. Oh crap, what a freak show of race conditions. 
Now, X::Worker could be run later in a so-called "X::run()" function. 
Worker = std::thread(...) _after_ the ctor is guaranteed to be 100% 
complete...

Launching a thread in a ctor of object A can and will start operating on 
the state of object A before the ctor of A is finished...

Is that kosher? Puke!

[toc] | [prev] | [next] | [standalone]

#85924

Am 14.08.22 um 21:00 schrieb Chris M. Thomasson:
> Launching a thread in a ctor of object A can and will start operating on 
> the state of object A before the ctor of A is finished...

The thread is kicked explicitly in the last line of the ctor after 
several other initializations and the class is internal and has no sub 
classes. It could safely start before the ctor really returned.


Marcel

[toc] | [prev] | [next] | [standalone]

#85926

On 8/14/2022 3:08 PM, Marcel Mueller wrote:
> Am 14.08.22 um 21:00 schrieb Chris M. Thomasson:
>> Launching a thread in a ctor of object A can and will start operating 
>> on the state of object A before the ctor of A is finished...
> 
> The thread is kicked explicitly in the last line of the ctor after 
> several other initializations and the class is internal and has no sub 
> classes. It could safely start before the ctor really returned.

Well, starting threads in the ctor is still a bit scary too me... ;^) I 
have seen shit like:

// quick and dirty pseudo-code

struct foo
{
    int a;

    foo()
    {
        // launch a thread using foo_thread(this);

        a = 42;
    }

    static void foo_thread(foo* self)
    {
       int sa = self->a;
       assert(sa == 42);
    }
}


foo::foo_thread can start up and read foo::a before the ctor was 
finished. I was asked why can some foo threads not see foo::a as 42? Yikes!

[toc] | [prev] | [next] | [standalone]

#85930

14.08.2022 22:00 Chris M. Thomasson kirjutas:
> On 8/14/2022 5:01 AM, Paavo Helde wrote:
>> 14.08.2022 12:50 Marcel Mueller kirjutas:
>>> In following code fragment a thread object is detached from a class 
>>> instance. (last line)
>>>
>>> class X
>>> {
>>>    std::mutex Lock;
>>>    std::thread Worker;
>>>    std::condition_variable CV;
>>
>> These two are in the wrong order. The thread member must be the last 
>> one, otherwise it may start running and access CV before CV is even 
>> created.
> [...]
> 
> Please, try not to create threads in an objects ctor when the thread is 
> going to start working on the state of the object as soon as its up and 
> running. I have had to debug full blown nightmares from other peoples 
> code using that horror. Oh crap, what a freak show of race conditions. 
> Now, X::Worker could be run later in a so-called "X::run()" function. 
> Worker = std::thread(...) _after_ the ctor is guaranteed to be 100% 
> complete...
> 
> Launching a thread in a ctor of object A can and will start operating on 
> the state of object A before the ctor of A is finished...
> 
> Is that kosher? Puke!


It's perfectly safe as long as the thread is the last member in the 
class and the constructor body is empty. My constructor bodies often are.

By artificially delaying the start of the thread you just introduce 
extra state and complicate the program, for no good reason.

[toc] | [prev] | [next] | [standalone]

#85931

On 8/14/2022 11:38 PM, Paavo Helde wrote:
> 14.08.2022 22:00 Chris M. Thomasson kirjutas:
>> On 8/14/2022 5:01 AM, Paavo Helde wrote:
>>> 14.08.2022 12:50 Marcel Mueller kirjutas:
>>>> In following code fragment a thread object is detached from a class 
>>>> instance. (last line)
>>>>
>>>> class X
>>>> {
>>>>    std::mutex Lock;
>>>>    std::thread Worker;
>>>>    std::condition_variable CV;
>>>
>>> These two are in the wrong order. The thread member must be the last 
>>> one, otherwise it may start running and access CV before CV is even 
>>> created.
>> [...]
>>
>> Please, try not to create threads in an objects ctor when the thread 
>> is going to start working on the state of the object as soon as its up 
>> and running. I have had to debug full blown nightmares from other 
>> peoples code using that horror. Oh crap, what a freak show of race 
>> conditions. Now, X::Worker could be run later in a so-called 
>> "X::run()" function. Worker = std::thread(...) _after_ the ctor is 
>> guaranteed to be 100% complete...
>>
>> Launching a thread in a ctor of object A can and will start operating 
>> on the state of object A before the ctor of A is finished...
>>
>> Is that kosher? Puke!
> 
> 
> It's perfectly safe as long as the thread is the last member in the 
> class and the constructor body is empty. My constructor bodies often are.

Most of the nightmare race condition occurred in a constructor body.

> 
> By artificially delaying the start of the thread you just introduce 
> extra state and complicate the program, for no good reason.

It's out of habit. I have always liked break the data-structures in a 
multi-threaded program apart, something like:

// pseudo code

struct foo
{
   int a;
   int b;
   int c;
};


struct bar
{
   int a;
   int b;
   int c;
};


struct worker_shared
{
   foo m_foo;
   bar m_bar;
};


struct worker_local
{
   foo m_foo;
   bar m_bar;
};


void worker_thread(worker_shared& shared)
{
    worker_local local;

    // Shared is shared. Local is, local... ;^)
}


void launch()
{
   worker_shared shared;
   std::thread worker(worker_thread, std::ref(shared));
   worker.join();
}


It helps me design things.

[toc] | [prev] | [next] | [standalone]

#85936

"Chris M. Thomasson" <chris.m.thomasson.1@gmail.com> writes:
>On 8/14/2022 5:01 AM, Paavo Helde wrote:
>> 14.08.2022 12:50 Marcel Mueller kirjutas:
>>> In following code fragment a thread object is detached from a class 
>>> instance. (last line)
>>>
>>> class X
>>> {
>>>    std::mutex Lock;
>>>    std::thread Worker;
>>>    std::condition_variable CV;
>> 
>> These two are in the wrong order. The thread member must be the last 
>> one, otherwise it may start running and access CV before CV is even 
>> created.
>[...]
>
>Please, try not to create threads in an objects ctor when the thread is 
>going to start working on the state of the object as soon as its up and 
>running. I have had to debug full blown nightmares from other peoples 
>code using that horror. Oh crap, what a freak show of race conditions. 
>Now, X::Worker could be run later in a so-called "X::run()" function. 
>Worker = std::thread(...) _after_ the ctor is guaranteed to be 100% 
>complete...
>
>Launching a thread in a ctor of object A can and will start operating on 
>the state of object A before the ctor of A is finished...

That's very easy to handle.   The run function waits on a condition
variable and the code that creates the object which contains the thread
signals the condition variable after the object creation is complete.

We do that all the time.

c_processor::c_processor(c_system *sp, c_logger *logger,
                         processor_number_t procnum, bool bootstrap)
    : c_thread("Processor #NN", logger),
      p_imask(this),
      p_interrupts(this),
      p_active_env(this),
      p_mop(this),
      p_toggles(this)
{
    char  pname[64];
    snprintf(pname, sizeof(pname), "Processor #%02u", (unsigned)procnum);
    set_threadname(pname);

    lock_thread();

    pthread_cond_init(&p_wait, NULL);
    pthread_cond_init(&p_wait_for_stop, NULL);
    pthread_cond_init(&p_wait_for_run, NULL);
    pthread_cond_init(&p_idle_wait, NULL);
    
    p_logger = logger;
    p_void = sp;
    p_mp = c_memory::self();
    p_procnum = procnum;
    p_bsp = bootstrap;
    p_stopped  = false;
    p_stop_request = true;
    p_pending_stop = c_mp::MAX_STOP_CODE;
    p_bl = NULL;
    p_kernel_bl = NULL;

    for(int i=1; i < 8; i++) {
        p_ix[i] = NULL;
    }

    p_tracebuffer_size = 132ul * 1000;
    p_tracebuffer = (char *)malloc(p_tracebuffer_size);
    if (p_tracebuffer == NULL) {
        p_logger->log("Processor %lu: Unable to allocate %zu bytes"
                      " for trace buffer\n"
                      "Tracing will be disabled\n", procnum, p_tracebuffer_size);
        p_tracebuffer_size = 0;
    } else {
        memset(p_tracebuffer, ' ', p_tracebuffer_size);
    }
    p_nexttrace = p_tracebuffer;
    reset();

    mp->set_thread_affinity(c_mp::PROCESSOR, this);

    thread_initialized();
    unlock_thread();
}

The call to 'thread_initialized' signals a condition variable at the start
of the thread ::run() method.

/**
 * Static pthread start function.   Invokes thread specific run virtual
 * function.
 *
 * @param arg   The class object pointer passed in from the pthread_create.
 */
void *
c_thread::run(void *arg)
{
    c_thread *tp = (c_thread *)arg;
    int       diag;

    pthread_mutex_lock(&tp->t_threadlock);
    while (!tp->t_thread_initialized) {
        pthread_cond_wait(&tp->t_ready, &tp->t_threadlock);
    }
    pthread_mutex_unlock(&tp->t_threadlock);

    diag = prctl(PR_SET_NAME, tp->t_thread_name, NULL, NULL, NULL, NULL);
    if (diag == -1) {
        tp->t_logger->log("Unable to set thread name: %s\n", strerror(errno));
    }

    tp->t_running = true;
    tp->run();
    tp->t_running = false;

    return NULL;
}

[toc] | [prev] | [next] | [standalone]

#85939

On 8/15/2022 8:31 AM, Scott Lurndal wrote:
> "Chris M. Thomasson" <chris.m.thomasson.1@gmail.com> writes:
>> On 8/14/2022 5:01 AM, Paavo Helde wrote:
>>> 14.08.2022 12:50 Marcel Mueller kirjutas:
>>>> In following code fragment a thread object is detached from a class
>>>> instance. (last line)
>>>>
>>>> class X
>>>> {
>>>>     std::mutex Lock;
>>>>     std::thread Worker;
>>>>     std::condition_variable CV;
>>>
>>> These two are in the wrong order. The thread member must be the last
>>> one, otherwise it may start running and access CV before CV is even
>>> created.
>> [...]
>>
>> Please, try not to create threads in an objects ctor when the thread is
>> going to start working on the state of the object as soon as its up and
>> running. I have had to debug full blown nightmares from other peoples
>> code using that horror. Oh crap, what a freak show of race conditions.
>> Now, X::Worker could be run later in a so-called "X::run()" function.
>> Worker = std::thread(...) _after_ the ctor is guaranteed to be 100%
>> complete...
>>
>> Launching a thread in a ctor of object A can and will start operating on
>> the state of object A before the ctor of A is finished...
> 
> That's very easy to handle.   The run function waits on a condition
> variable and the code that creates the object which contains the thread
> signals the condition variable after the object creation is complete.
> 
> We do that all the time.
[ snip code ]

That will work as well. :^)

Still, my experience with having to debug race conditions in ctors might 
of damaged me in a way... I always try not to launch threads in a ctor 
that operate on the object being constructed. Damaged goods... ;^o

Actually, some of the worst bugs have had to correct is when an object 
derives from a "thread object" that starts a thread that ends up calling 
a virtual function into the object. Is that ctor of the derived object 
done yet? Even though I was sometimes paid to do it, it left mental 
scars in my brain. Yikes!

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.c++

csiph-web