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

Groups > comp.lang.c > #103784 > unrolled thread

Memory not updating in thread as expected

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

Contents

  Memory not updating in thread as expected Test <test@.nil.invalid.com> - 2017-02-13 12:46 +0200
    Re: Memory not updating in thread as expected m.labanowicz@gmail.com - 2017-02-13 05:18 -0800
      Re: Memory not updating in thread as expected Test <test@.nil.invalid.com> - 2017-02-13 16:25 +0200
        Re: Memory not updating in thread as expected m.labanowicz@gmail.com - 2017-02-13 10:30 -0800
          Re: Memory not updating in thread as expected Jerry Stuckle <jstucklex@attglobal.net> - 2017-02-13 14:30 -0500
    Re: Memory not updating in thread as expected m.labanowicz@gmail.com - 2017-02-13 05:46 -0800
      Re: Memory not updating in thread as expected Ben Bacarisse <ben.usenet@bsb.me.uk> - 2017-02-13 14:30 +0000
        Re: Memory not updating in thread as expected "Chris M. Thomasson" <invalid@invalid.invalid> - 2017-02-13 15:13 -0800
      Re: Memory not updating in thread as expected Test <test@.nil.invalid.com> - 2017-02-13 16:36 +0200
        Re: Memory not updating in thread as expected David Brown <david.brown@hesbynett.no> - 2017-02-13 23:15 +0100
          Re: Memory not updating in thread as expected James Kuyper <jameskuyper@verizon.net> - 2017-02-13 22:04 -0500
            Re: Memory not updating in thread as expected David Brown <david.brown@hesbynett.no> - 2017-02-14 09:44 +0100
              Re: Memory not updating in thread as expected Ben Bacarisse <ben.usenet@bsb.me.uk> - 2017-02-14 11:07 +0000
                Re: Memory not updating in thread as expected David Brown <david.brown@hesbynett.no> - 2017-02-14 12:46 +0100
                  Re: Memory not updating in thread as expected Robert Wessel <robertwessel2@yahoo.com> - 2017-02-16 05:12 -0600
                    Re: Memory not updating in thread as expected David Brown <david.brown@hesbynett.no> - 2017-02-16 12:44 +0100
                Re: Memory not updating in thread as expected Alain Ketterlin <alain@universite-de-strasbourg.fr.invalid> - 2017-02-14 15:53 +0100
                  Re: Memory not updating in thread as expected Ben Bacarisse <ben.usenet@bsb.me.uk> - 2017-02-14 15:36 +0000
                    Re: Memory not updating in thread as expected Alain Ketterlin <alain@universite-de-strasbourg.fr.invalid> - 2017-02-14 17:58 +0100
                      Re: Memory not updating in thread as expected Ben Bacarisse <ben.usenet@bsb.me.uk> - 2017-02-14 17:26 +0000
            Re: Memory not updating in thread as expected Alain Ketterlin <alain@universite-de-strasbourg.fr.invalid> - 2017-02-14 10:43 +0100
              Re: Memory not updating in thread as expected Robert Wessel <robertwessel2@yahoo.com> - 2017-02-14 03:52 -0600
                Re: Memory not updating in thread as expected Alain Ketterlin <alain@universite-de-strasbourg.fr.invalid> - 2017-02-14 11:36 +0100
                Re: Memory not updating in thread as expected supercat@casperkitty.com - 2017-02-14 05:31 -0800
    Re: Memory not updating in thread as expected Ken Brody <kenbrody@spamcop.net> - 2017-02-13 12:44 -0500
    Re: Memory not updating in thread as expected Joe Pfeiffer <pfeiffer@cs.nmsu.edu> - 2017-02-13 10:58 -0700
    Re: Memory not updating in thread as expected "Chris M. Thomasson" <invalid@invalid.invalid> - 2017-02-14 15:13 -0800
      Re: Memory not updating in thread as expected "Chris M. Thomasson" <invalid@invalid.invalid> - 2017-02-14 16:37 -0800

Page 2 of 2 — ← Prev page 1 [2]

#103862

James Kuyper <jameskuyper@verizon.net> writes:

> You sound like you have some idea how to do this kind of thing (which is
> more than I could say about myself). I'd prefer (particularly in this
> newsgroup) sticking strictly to C2011's new threading library, rather
> than making any use of anything Linux-specific. Could you (or anyone
> else who cares to do so) show us the right way to do something like the
> OP's original code?

Here is the setting, as far as I remember: one thread does some work and
then "wakes up" another, that then has other work to do:

    work1();
    signal    ---->  wait
                     work2();

where work2() cannot start before work1() is finished.

You need two main things: one is a (set of) regular variables indicating
the work is done. OP used an array element, I'll use a single boolean

    bool ready = false; // init at the start of program

(use whatever is needed to represent the logical conditions: it could be
a set of variables or array elements...)

Then you need synchronization objects: I'll use a condition variable
(and therefore an accompanying mutex):

    mtx_t m;
    cnd_t c;

The mutex will protect concurrent accesses to "ready", and the condition
variable will provide the signaling mechanism. Both need to be properly
initialized before any use.

Now here is what thread 1 must do:

    work1();
    mtx_lock(&m);
    ready = true;
    cnd_signal(&c);
    mtx_unlock(&m);

It is not mandatory that cnd_signal() is included inside
mtx_lock()/mtx_unlock() I think (not sure about this---when I learned
condition variables on Solaris the _signal had to be protected).

Now here is what thread 2 must do:

    mtx_lock(&m);
    while ( ready == false )
        cnd_wait(&c,&m);
    mtx_unlock(&m);
    work2();

In the general case, thread 2 must loop, but here the "while" could be
an "if", because the synchronization is so simple. Anyway, cnd_wait()
avoids the (disastrous) busy-wait the OP used.

Now this works whatever the executions interleaving is.

1) Suppose thread 1 is very fast and completes (i.e., signals) before
   thread 2 even reaches the mtx_lock(): in that case, thread 2 will
   later lock the mutex, find "ready == true", immediately unlock the
   mutex and proceed.

2) Suppose thread 2 is very fast: it will lock the mutex, enter the loop
   and block in cnd_wait() until someone signals on c, leaving the mutex
   unlocked (yes, cnd_wait() does all this at once). Thread 2 will stay
   blocked until thread 1 calls cnd_signal(). At that point, thread 2 is
   waken up but waits inside cnd_wait() for the mutex to be unlocked,
   which will only happen when thread 1 calls mtx_unlock(). When this
   happens thread 2 holds the mutex, and can test its logical condition
   ("ready == false") again, exit the loop, unlock the mutex, and
   proceed.

3) If both threads reach mtx_lock() at about the same time, only one of
   them will succeed in mtx_lock(). Which one is the winner determines
   on whether we are back to case 1) or 2).

Note that this works only for a single signal-wait cycle. If this cycle
is to be repeated, more complex synchro is needed (essentially, thread2
has to "acknowledge" receipt of the signal, reset the "ready" variable,
etc.)

Note also that the whole thing would be much simpler with a semaphore.
C11 has no semaphores afaik. Using a POSIX semaphore s, the code would
be:

    (thread 1)
    work1();
    sem_post(&s);

    (thread 2)
    sem_wait(&s);
    work2();

> He didn't provide a complete program, or any context for that code, so
> feel free to make reasonable choices about what the context would
> actually look like, without making it excessively complicated. str1[0]
> seems to be being used solely for the purpose of synchronizing
> execution of the two threads, and the rest of the allocated memory is
> not used at all, so feel free to drop the allocated memory entirely,
> and use the most appropriate method to achieve that synchronization.

That's what I did: the "ready" boolean above plays the role of
str1[0] being equal to 50 (or whatever).

> I'm not just asking this on the OP's behalf - I've little experience
> with this kind of code, and none of it recent, and therefore none of
> it using C2011 - so I'd like to see it for myself.

If you have used pthread's library functions, the correspondance is
immediate:

    mtx_lock() <-> pthread_mutex_lock()
    ...
    cnd_signal() <-> pthread_cond_signal()
    ...

-- Alain.

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

#103863

On Tue, 14 Feb 2017 10:43:13 +0100, Alain Ketterlin
<alain@universite-de-strasbourg.fr.invalid> wrote:

>James Kuyper <jameskuyper@verizon.net> writes:
>
>> You sound like you have some idea how to do this kind of thing (which is
>> more than I could say about myself). I'd prefer (particularly in this
>> newsgroup) sticking strictly to C2011's new threading library, rather
>> than making any use of anything Linux-specific. Could you (or anyone
>> else who cares to do so) show us the right way to do something like the
>> OP's original code?
>
>Here is the setting, as far as I remember: one thread does some work and
>then "wakes up" another, that then has other work to do:
>
>    work1();
>    signal    ---->  wait
>                     work2();
>
>where work2() cannot start before work1() is finished.
>
>You need two main things: one is a (set of) regular variables indicating
>the work is done. OP used an array element, I'll use a single boolean
>
>    bool ready = false; // init at the start of program
>
>(use whatever is needed to represent the logical conditions: it could be
>a set of variables or array elements...)
>
>Then you need synchronization objects: I'll use a condition variable
>(and therefore an accompanying mutex):
>
>    mtx_t m;
>    cnd_t c;
>
>The mutex will protect concurrent accesses to "ready", and the condition
>variable will provide the signaling mechanism. Both need to be properly
>initialized before any use.
>
>Now here is what thread 1 must do:
>
>    work1();
>    mtx_lock(&m);
>    ready = true;
>    cnd_signal(&c);
>    mtx_unlock(&m);
>
>It is not mandatory that cnd_signal() is included inside
>mtx_lock()/mtx_unlock() I think (not sure about this---when I learned
>condition variables on Solaris the _signal had to be protected).
>
>Now here is what thread 2 must do:
>
>    mtx_lock(&m);
>    while ( ready == false )
>        cnd_wait(&c,&m);
>    mtx_unlock(&m);
>    work2();
>
>In the general case, thread 2 must loop, but here the "while" could be
>an "if", because the synchronization is so simple. Anyway, cnd_wait()
>avoids the (disastrous) busy-wait the OP used.
>
>Now this works whatever the executions interleaving is.
>
>1) Suppose thread 1 is very fast and completes (i.e., signals) before
>   thread 2 even reaches the mtx_lock(): in that case, thread 2 will
>   later lock the mutex, find "ready == true", immediately unlock the
>   mutex and proceed.
>
>2) Suppose thread 2 is very fast: it will lock the mutex, enter the loop
>   and block in cnd_wait() until someone signals on c, leaving the mutex
>   unlocked (yes, cnd_wait() does all this at once). Thread 2 will stay
>   blocked until thread 1 calls cnd_signal(). At that point, thread 2 is
>   waken up but waits inside cnd_wait() for the mutex to be unlocked,
>   which will only happen when thread 1 calls mtx_unlock(). When this
>   happens thread 2 holds the mutex, and can test its logical condition
>   ("ready == false") again, exit the loop, unlock the mutex, and
>   proceed.
>
>3) If both threads reach mtx_lock() at about the same time, only one of
>   them will succeed in mtx_lock(). Which one is the winner determines
>   on whether we are back to case 1) or 2).
>
>Note that this works only for a single signal-wait cycle. If this cycle
>is to be repeated, more complex synchro is needed (essentially, thread2
>has to "acknowledge" receipt of the signal, reset the "ready" variable,
>etc.)
>
>Note also that the whole thing would be much simpler with a semaphore.
>C11 has no semaphores afaik. Using a POSIX semaphore s, the code would
>be:
>
>    (thread 1)
>    work1();
>    sem_post(&s);
>
>    (thread 2)
>    sem_wait(&s);
>    work2();
>
>> He didn't provide a complete program, or any context for that code, so
>> feel free to make reasonable choices about what the context would
>> actually look like, without making it excessively complicated. str1[0]
>> seems to be being used solely for the purpose of synchronizing
>> execution of the two threads, and the rest of the allocated memory is
>> not used at all, so feel free to drop the allocated memory entirely,
>> and use the most appropriate method to achieve that synchronization.
>
>That's what I did: the "ready" boolean above plays the role of
>str1[0] being equal to 50 (or whatever).
>
>> I'm not just asking this on the OP's behalf - I've little experience
>> with this kind of code, and none of it recent, and therefore none of
>> it using C2011 - so I'd like to see it for myself.
>
>If you have used pthread's library functions, the correspondance is
>immediate:
>
>    mtx_lock() <-> pthread_mutex_lock()
>    ...
>    cnd_signal() <-> pthread_cond_signal()
>    ...


With the caveat that I've never actually used the C11 functions:

A simple completion flag like the OP was using can be trivially
implemented "correctly" with some of the stuff in stdatomic.h.  Create
it, then set/test it with atomic_store() and atomic_load().  Those
explicitly provide sequential consistency.

No whether the design is a good one or not, I can't say (and I really
have no idea what the array of 50 items is doing), but things that
poll are often suspect.

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

#103864

Robert Wessel <robertwessel2@yahoo.com> writes:
[...]
>>If you have used pthread's library functions, the correspondance is
>>immediate:
>>
>>    mtx_lock() <-> pthread_mutex_lock()
>>    ...
>>    cnd_signal() <-> pthread_cond_signal()
>>    ...
>
> With the caveat that I've never actually used the C11 functions:
>
> A simple completion flag like the OP was using can be trivially
> implemented "correctly" with some of the stuff in stdatomic.h.  Create
> it, then set/test it with atomic_store() and atomic_load().  Those
> explicitly provide sequential consistency.

I see what you mean, but I think the problem is not so much with
"correctness" of the concurrent accesses than with the busy-waiting.
Even volatile would force the compiler to insert memory fences, and
ensure consistency.

> No whether the design is a good one or not, I can't say (and I really
> have no idea what the array of 50 items is doing), but things that
> poll are often suspect.

I fully agree.

-- Alain.

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

#103879

On Tuesday, February 14, 2017 at 3:52:39 AM UTC-6, robert...@yahoo.com wrote:
> No whether the design is a good one or not, I can't say (and I really
> have no idea what the array of 50 items is doing), but things that
> poll are often suspect.

In some contexts, especially in the embedded world, what's important is to
avoid blocking.  An attempt to do something may immediately succeed, fail, or 
report a "not ready" condition, but in the latter case code must proceed on
to other things and then come back to retry the action later.  Embedded code
generally uses an interrupt model which is a bit different from the threading
model used in typical operating systems: once an interrupt starts executing,
it must run to completion before control returns to the main thread.  If
something isn't ready for the interrupt to process it, it may be able to set
up another interrupt to happen in the future (or interrupts may be set to
fire on some scheduled basis) but it's absolutely vital that the interrupt
not block.

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

#103820

On 2/13/2017 5:46 AM, Test wrote:
> In my (linux) c code I create a thread which allocates a string, say:
>
> .. in the thread function:
> str1=alloc(50);
> str1[0]=50;
> while (str1[0]==50)
>   {
>   sleep(1);
>   }
> str1[0]=2; // signal main program that I'm out of teh loop and about to finish
> return;
> }

Might this be an optimization issue, requiring you to make sure you define 
str1 as being a pointer to "volatile char"?

-- 
Kenneth Brody

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

#103823

Test <test@.nil.invalid.com> writes:

Let's label the line numbers:

t1: str1=alloc(50);
t2: str1[0]=50;
t3: while (str1[0]==50)
    {
t4:   sleep(1);
    }
t5: tr1[0]=2; // signal main program that I'm out of teh loop and about to finish
t6: return;
 } 
> // str2 has the address of str1 in the thread, ie. (long)str1==(long)str2
m1: str2[0]=0;  // signal thread that it should exit the loop
m2: printf("%d\n",(int)str2[0]);   // "0" is shown
m3: // {int i11;for (i11=0;i11<8;i11++) printf("[%.2X]",str2[i11]);printf("\n");}
m4: while (str2[0]==0)
    { // waiting for the thread to signal it's about to finish 
m5:    printf("%d\n",(int)str2[0]);   // "50" is shown WTF??
m6:    sleep(1);
>    }
m7: printf("Done %d\n",(int)str2[0]);   // "50" is shown WTF??

Your code can easily execute in the order
t1 m1 m2 m3 m4 t2 m5

(even in the absence of issues with volatile keywords or optimization)

>
> When the main program loop started str2[0] was zero but suddenly changed to 50.
> The thread is not changing it within its loop.
> What gives? It works if I uncomment the line "{int i11;for (i1..." and get:
> 0
> [00][00][00][00][00][00][00][00]
> ..on the screen

-- 
"Erwin, have you seen the cat?" -- Mrs. Shrödinger

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

#103910

On 2/13/2017 2:46 AM, Test wrote:
> In my (linux) c code I create a thread which allocates a string, say:
>
> .. in the thread function:
> str1=alloc(50);
> str1[0]=50;
> while (str1[0]==50)
>   {
>   sleep(1);
>   }
> str1[0]=2; // signal main program that I'm out of teh loop and about to finish
> return;
> }
[...]

Fwiw, here is a crude C11 thing, slightly related to your example code 
wrt logic and "worker" thread exit, that compiles for me only due to a 
little layer built on top of PThreads:

https://groups.google.com/d/topic/comp.lang.c/WPH7ed5uS4Y/discussion

This had to be completed simply because my compiler set does not support 
C11 threading.

Ummm, I think printf is thread safe of some things... Ummm, mabey, okay? 
Shit.

Anyway, here is the code:
___________________________________________________
#include "threads.h"

// little hack, because I forgot to add thrd_yield
// in threads.h emulation layer
#define thrd_yield sched_yield


#include <stdlib.h>
#include <stdio.h>


#define ERROR 1


struct shared
{
     mtx_t mtx;
     cnd_t cond;
     int state;
};


int shared_create(struct shared* self)
{
     self->state = 0;

     if (mtx_init(&self->mtx, ct_mtx_plain) == thrd_success)
     {
         if (cnd_init(&self->cond) == thrd_success)
         {
             return 0;
         }
     }

     return ERROR;
}

void shared_destroy(struct shared* self)
{
     cnd_destroy(&self->cond);
     mtx_destroy(&self->mtx);
}



int my_thread_entry(void* sraw)
{
     struct shared* const self = sraw;

     printf("my_thread_entry(%p)\n", (void*)self);

     mtx_lock(&self->mtx);

     while (!self->state)
         cnd_wait(&self->cond, &self->mtx);

     mtx_unlock(&self->mtx);

     printf("my_thread_exit(%p)\n", (void*)self);

     return 123;
}


int main()
{
     {
         thrd_t my_thread;
         struct shared share;

         if (!shared_create(&share))
         {
             if (thrd_create(&my_thread, my_thread_entry, &share) == 
thrd_success)
             {
                 thrd_yield();
                 thrd_yield();
                 thrd_yield();

                 mtx_lock(&share.mtx);
                 share.state = 1;
                 mtx_unlock(&share.mtx);
                 cnd_signal(&share.cond);


                 int status = 0;
                 thrd_join(my_thread, &status);

                 printf("status = %d\n", status);
             }

             shared_destroy(&share);
         }
     }

     printf("\ncomplete! Hit <enter> to exit...\n");
     fflush(stdout);
     getchar();

     return 0;
}
___________________________________________________


Now, this is fine as a base for modeling higher performing alternatives, 
perhaps a little lock-free spice? Well, now. What is the purpose of 
mutating a certain location of an array anyway?

Think about the visibility requirements of your synchronization logic ...

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

#103916

On 2/14/2017 3:13 PM, Chris M. Thomasson wrote:
> On 2/13/2017 2:46 AM, Test wrote:
>> In my (linux) c code I create a thread which allocates a string, say:
>>
>> .. in the thread function:
>> str1=alloc(50);
>> str1[0]=50;
>> while (str1[0]==50)
>>   {
>>   sleep(1);
>>   }
>> str1[0]=2; // signal main program that I'm out of teh loop and about
>> to finish
>> return;
>> }
> [...]
>
> Fwiw, here is a crude C11 thing, slightly related to your example code
> wrt logic and "worker" thread exit, that compiles for me only due to a
> little layer built on top of PThreads:
>
> https://groups.google.com/d/topic/comp.lang.c/WPH7ed5uS4Y/discussion
>
> This had to be completed simply because my compiler set does not support
> C11 threading.
>
> Ummm, I think printf is thread safe of some things... Ummm, mabey, okay?
> Shit.
>
> Anyway, here is the code:
> ___________________________________________________
> #include "threads.h"
>
> // little hack, because I forgot to add thrd_yield
> // in threads.h emulation layer
> #define thrd_yield sched_yield
>
>
> #include <stdlib.h>
> #include <stdio.h>
>
>
> #define ERROR 1
>
>
> struct shared
> {
>     mtx_t mtx;
>     cnd_t cond;
>     int state;
> };
>
>
> int shared_create(struct shared* self)
> {
>     self->state = 0;
>
>     if (mtx_init(&self->mtx, ct_mtx_plain) == thrd_success)
>     {
>         if (cnd_init(&self->cond) == thrd_success)
>         {
>             return 0;
>         }
^^^^^^^^^^^^^^^^^^^^^^^^^

Yikes! Forgot to destroy the fully created self->mtx here. Should have this:

           mtx_destroy(&self->mtx);

damn it.


>     }
>
>     return ERROR;
> }
>
> void shared_destroy(struct shared* self)
> {
>     cnd_destroy(&self->cond);
>     mtx_destroy(&self->mtx);
> }
>
>
>
> int my_thread_entry(void* sraw)
> {
>     struct shared* const self = sraw;
>
>     printf("my_thread_entry(%p)\n", (void*)self);
>
>     mtx_lock(&self->mtx);
>
>     while (!self->state)
>         cnd_wait(&self->cond, &self->mtx);
>
>     mtx_unlock(&self->mtx);
>
>     printf("my_thread_exit(%p)\n", (void*)self);
>
>     return 123;
> }
>
>
> int main()
> {
>     {
>         thrd_t my_thread;
>         struct shared share;
>
>         if (!shared_create(&share))
>         {
>             if (thrd_create(&my_thread, my_thread_entry, &share) ==
> thrd_success)
>             {
>                 thrd_yield();
>                 thrd_yield();
>                 thrd_yield();
>
>                 mtx_lock(&share.mtx);
>                 share.state = 1;
>                 mtx_unlock(&share.mtx);
>                 cnd_signal(&share.cond);
>
>
>                 int status = 0;
>                 thrd_join(my_thread, &status);
>
>                 printf("status = %d\n", status);
>             }
>
>             shared_destroy(&share);
>         }
>     }
>
>     printf("\ncomplete! Hit <enter> to exit...\n");
>     fflush(stdout);
>     getchar();
>
>     return 0;
> }
> ___________________________________________________
>
>
> Now, this is fine as a base for modeling higher performing alternatives,
> perhaps a little lock-free spice? Well, now. What is the purpose of
> mutating a certain location of an array anyway?
>
> Think about the visibility requirements of your synchronization logic ...
>

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

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

csiph-web