2

I've been reference this link -> https://gist.github.com/Taymindis/3938e917aaae4fc480386f494be62f0e and do some valgrind check, it has no error. But I just want to double confirm is this example below consider thread safe?

I've personally vagrind check, it has no error, Does anyone has any better idea?

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

#define THREAD_RUN 100

static char *global;

static char *x1 = "This is thread 1";

static char *x2 = "This is thread 2";

void * write(void* thr_data) {
    int n = *(int*)thr_data;

    if(n%2==0) goto RETRY1;
    else goto RETRY2;

RETRY1:
    for (n = 0; n < 1000; ++n) {
        global = x1;
    }
    goto RETRY1;


RETRY2:
    for (n = 0; n < 1000; ++n) {
        global = x2;
    }
    goto RETRY2;

    // free(cu);
    return 0;
}

void * read(void* thr_data)
{
    int n;
RETRY:
    for (n = 0; n < 1000; ++n) {
        if(global[0] == 'C');
    }
    goto RETRY;
    return 0;
}


int main(void)
{
    int n;
    pthread_t read_thr[THREAD_RUN];
    pthread_t write_thr[THREAD_RUN];

    global = x1;

    for (n = 0; n < THREAD_RUN; ++n) {
        pthread_create(&write_thr[n], NULL, write, &n);
        pthread_create(&read_thr[n], NULL, read, &n);
    }

    for (n = 0; n < THREAD_RUN; ++n)
        pthread_join(read_thr[n], NULL);

    for (n = 0; n < THREAD_RUN; ++n)
        pthread_join(write_thr[n], NULL);

} 
minika woon
  • 68
  • 1
  • 5
  • On the contrary I believe someone came up with this example to show you how unpredictable the switch between them might be. – patatahooligan Sep 25 '17 at 14:45
  • I can't understand, I am using valgrind to run this for 2 mins, no error at all. What is mean is 2 predefined(pre-initialized) variable with pointer swapping only. – minika woon Sep 25 '17 at 14:47
  • The pointer is a variable. Also, undefined behavior can be anything, including what you initially intended. – patatahooligan Sep 25 '17 at 14:48

3 Answers3

6

No, pointer assignment is not guaranteed by either C or C++ to be atomic.

(It's perfectly conceivable, for example, that a pointer spans two registers, and you end up with a mash-up of x1 and x2).

Your code is not thread-safe.

Bathsheba
  • 231,907
  • 34
  • 361
  • 483
  • For this case, what will it cause? memory corrupt? Because it is just pointer by pointer assignment – minika woon Sep 25 '17 at 14:42
  • 2
    That's not the only problem. Because the variable is neither atomic nor volatile, the compiler might do any optimization it chooses, including only writing once in each thread and then going in an infinite no-op loop. – patatahooligan Sep 25 '17 at 14:44
  • what if i change to `volatile static char *global;` – minika woon Sep 25 '17 at 15:20
  • 1
    @minikawoon see [here](https://stackoverflow.com/questions/4557979/when-to-use-volatile-with-multi-threading) – alain Sep 25 '17 at 15:43
1

Well, Based on the coding logic regardless infinite loop, if you concern about memory corruption, I will said “no I won’t be happen due to assignment point is eventually assigned x1 or x2.

I think you are not clear enough what is not guarantee assignment is atomic, if your program has multiple thread doing assignment to one global variable, it will not guarantee you that each of assignment is successfully assigned to it. Based on that, it might have a lot of x1 assignment has lost assigned or the other one.

Back to what is your main purpose of this code, if your are planning to change pointer time to time and multiple read operation. I suggest you only 1 thread to do assignment

Oktaheta
  • 606
  • 5
  • 21
  • Yes, I'm only using 1 thread for assigning operation. I've set a timer every 5 minutes swap a new predefined pointer assign to global variable for new update. There are around 100k consumer to read the global variable and I did not see any issue as it is still under UAT. – minika woon Sep 26 '17 at 01:07
  • 1
    If it is only 1 thread to do assignment operation, then it is nothing atomic to talk about. I assumed that there is not memory management in your code, if you’re swapping pointer meanwhile old pointer is still holding by other thread, it will causing UB. I suggested you to using some c lib to manage your project rather than implement yourself – Oktaheta Sep 26 '17 at 01:09
  • I am using https://github.com/Taymindis/binary_array and it has a function call `bin_array_safety_swap` https://github.com/Taymindis/binary_array#void-bin_array_safety_swapbin_array_t-curr-bin_array_t-new_a-free_node_fn-free_node_fn-unsigned-int-buffer_time_mic_sec – minika woon Sep 26 '17 at 01:18
0

No, this is not safe, it is a data race. It is undefined behavior. The point is not so much what happens on the hardware, but what happens inside the compiler. Compilers have become very clever at optimizing code, they try to prove certain properties of the code to be able to transform it to something that runs faster. To do this, they assume the program does not have undefined behavior.

As pointed out in a comment, it would be a valid and useful optimisation to replace

while(1) global = x1;

by

global = x1;

because the compiler will assume global is not changed by another thread.

alain
  • 11,939
  • 2
  • 31
  • 51