Accessing variable from two threads in C

Question

I have the following piece of code:

int attempts = 0;
while(ptr== NULL && attempts < 60) {
        sleep(1000);
        attempts++;
    }

that continuously loops waiting for the pointer to be set by another thread. The other thread will simply do

ptr = //some value

My question is, is this safe? Will this cause any memory corruption that lead to hard to debug error conditions later?

P.S: I'm aware that the compiler may optimise out the ptr due to the lack of the volatile keyword. That doesn't matter as much to me. I'm only concerned whether this will cause problems to other unrelated parts of the code.

Consider using atomic operations like described in this reference http://en.cppreference.com/w/c/atomic It was introduced in C11 specification. `atomic_compare_exchange_strong`is what you need in your loop. — bkausbk, Sep 02 '14 at 09:04
It seems like safe, assignment operations are atomic. But, the approach isn't sane, what if while you're sleep()-ing, ptr gets changed to some value and again changes back to NULL ? — rakib_, Sep 02 '14 at 09:09
The other thread will simply do - only one writing thread? If yes, it seems like safe because it's simple assembler instruction (mov on x86). If no, then it's not safe. — someuser, Sep 02 '14 at 09:19
@someuser Yes, there is only one writing thread, But, the processor is not x86. — rubndsouza, Sep 02 '14 at 09:21
If the write operation of the register value into memory takes one assembly instruction it is quite safe. — someuser, Sep 02 '14 at 09:30
@someuser: It is not always safe. Depends upon the processor (and its cache coherency mechanism with other cores) and the alignment (some processors can load an unaligned data but such loads are not atomic). — Basile Starynkevitch, Sep 02 '14 at 09:31
Sorry. I was wrong. Suppose the first thread loads the value of pointer to register. Second thread has time to change this value. But the first thread will compare the value recorded in the register before. Comparison will not work correctly. — someuser, Sep 02 '14 at 09:44
@BasileStarynkevitch I agree that comparision, can go wrong, but how can this cause a crash? I didnt understand the point about cache coherency and allignment — bobby, Sep 02 '14 at 09:59
@jogabonito Consider if the compiler optimizes the comparison of `ptr` to `NULL` by decrementing `ptr`, testing the high bit, and then incrementing `ptr` to restore it. If the change to `ptr` falls between those operations, `ptr` will be off by one and dereferencing it can crash. — David Schwartz, Sep 04 '14 at 09:37

David Schwartz · Answer 1 · 2014-09-04T09:34:59.530

This question is impossible to answer as asked. The C language has no support for threads. So if you have threads, you are getting that support from some standard or library that has to document what is safe and what is not. Most likely, the threading standard says that modifying an object on one thread while another thread is, or might be, accessing it is undefined behavior. In that case, it's definitely not safe. But you need to check the particular standard you are coding to.

By the way, you cannot determine whether or not it's safe by testing. It may appear safe or appear to do what you expect in your testing and then fail with a different OS version, different CPU, different compiler options, different BIOS settings, or even just due to bad luck.

Basile Starynkevitch · Accepted Answer · 2014-09-04T10:13:10.643

2

At the very least, ptr could be declared as volatile ; but that is not enough. You really want atomic operations. With C11 you have <stdatomic.h> standard header. And recent GCC has atomic builtins that you should use. Both the access and the writing (in "your other thread") of your ptr should be atomic!

Actually (without using atomic operations) the behavior that you would observe is undefined behavior and could vary a lot (with different processors, different compilers, etc...).

^{Sadly, on many x86 processors, you may not notice that UB}

You need the compiler to enforce/use cache coherence by emitting particular machine instructions.

You could also use condition variables with mutex locks, or some semaphore.

With a recent GCC (at least 4.9) you might consider compiling with -fsanitize=thread and/or -fsanitize=address (for debugging) if your target processor supports that.

BTW, your memory corruption might be completely unrelated. You could consider using valgrind which is supported on many platforms (it is better to compile your program with -g and you could try to compile with gcc -O1 -g if you wanted to).

I also recommend using recent tools (recent version 4.9 of GCC -in september 2014-, recent binutils, recent gdb, recent libc, recent kernel....)

edited Sep 04 '14 at 10:13

answered Sep 02 '14 at 08:54

Basile Starynkevitch

223,805
18
296
547

1

Not much of an answer, or an explanation (it is however a good alternative). Please elaborate. OP needs to understand why. – Mihai Stancu Sep 02 '14 at 08:54
My question is whether the code will cause any undefined behaviour. The fact that the compiler may optimise out the ptr due to the lack of the volatile keyword doesn't matter as much to me. – rubndsouza Sep 02 '14 at 08:57
Yes, your code will cause UB, but on most x86 machines you might not notice it.... – Basile Starynkevitch Sep 02 '14 at 08:59
IIRC Postgresql had a similar bug, which appeared only on non-x86 machines. But I can't find any reference now! – Basile Starynkevitch Sep 02 '14 at 09:06
@BasileStarynkevitch I'm facing a bug in a non-x86 machine, seems like heap corruption. Could this be the root cause? – rubndsouza Sep 02 '14 at 09:23
@rubndsouza: perhaps yes. But you really should correct your code by using atomic operations. Also, use a *recent* GCC compiler (e.g. 4.9.1 today), since multithreaded memory model is a tricky issue which has improved a lot in recent GCC compilers (and in recent C standards). Options like `-fsanitize=thread` and `-fsanitize=address` should help. – Basile Starynkevitch Sep 02 '14 at 09:24
2

Terrible answer. Since `volatile` is neither necessary nor sufficient, what's the point in using it? – David Schwartz Sep 04 '14 at 09:28

score 0 · Answer 3 · answered Sep 02 '14 at 09:03

0

As far as i know, NO. It is not safe and could cause inconsistent reads. Best way would be to protect the variable ptr with a mutex or a binary semaphore. Check mutex.h for mutex functions (such as mutex_init, mutex_lock, mutex_unlock etc).

answered Sep 02 '14 at 09:03

Shankar Narayanan

151
4

Accessing variable from two threads in C

3 Answers3

Linked