3

First of all, I have a gut feeling that says, inside an if statement, if I am using the variable, it counts as reading the variable so I should lock it with mutex there also (if another pthread might be doing stuff with it). Am I right that I should lock it?

The example situation in simplified manner is given below.

in one thread I am using the below statement:

if(event){
  // Should I or should I not lock event here to use it
  // inside if statement?
  pthread_mutex_lock(&mutex_event);
  event = 0;
  pthread_mutex_unlock(&mutex_event);
  // blah blah code here
  // blah blah code here
  // blah blah code here
}

in another thread I am doing

pthread_mutex_lock(&mutex_event);
event = 1;
pthread_mutex_unlock(&mutex_event);

Second question, if I do need to lock it, how should I do it in a classy programmers manner? In other words, what is the general convention. I don't like the below idea as I have to wait for all of the lines inside "if" to execute to be able to unlock mutex again.

pthread_mutex_lock(&mutex_event);
if(event){ 
  event = 0;
  // blah blah code here
  // blah blah code here
  // blah blah code here
}
pthread_mutex_unlock(&mutex_event);

I am ok with this idea, but probably it could look prettier:

pthread_mutex_lock(&mutex_event);
if(event){    
  event = 0;
  pthread_mutex_unlock(&mutex_event);
  // blah blah code here
  // blah blah code here
  // blah blah code here
}
else
{
  pthread_mutex_unlock(&mutex_event);
}

I find that it gets trickier with a while loop and this is the primitive solution I have got:

pthread_mutex_lock(&mutex_event);
store_event = event; // store_event is local
pthread_mutex_unlock(&mutex_event);
while(store_event){ 
  // blah blah code here
  // blah blah code here
  // blah blah code here
}
jotik
  • 17,044
  • 13
  • 58
  • 123
etugcey
  • 109
  • 2
  • 11

3 Answers3

4

Every access to the shared variable - write AND read - should be guarded. How much else you surround by mutexes is an individual matter - balancing the overhead, atomicity of using your variable and clarity of code.

Also, you don't want to spend a long time in the protected area, so if you synchronize a variable two times in a long section of code, you don't want to lock the whole big section restricted.

There's obviously a problem with some reads:

pthread_mutex_lock(&mutex_event);
if(event){ 
  event = 0;
  pthread_mutex_unlock(&mutex_event);
  // blah blah code here
  // blah blah code here
  // blah blah code here
}
else pthread_mutex_unlock(&mutex_event); //awful, a hundred lines below the opening.

It's much better in that case to leave distinct 'synchronization zones', and operate on a local copy of the variable.

pthread_mutex_lock(&mutex_event);
event_copy = event;
data_copy = data;
state_copy = state;
pthread_mutex_unlock(&mutex_event);

if(event_copy){ 
  // blah blah code here
  // blah blah code here
  event_copy = 0;
  // blah blah code here
}
// blah blah code here

pthread_mutex_lock(&mutex_event);
event = event_copy;
data = data_copy;
state = state_copy;
pthread_mutex_unlock(&mutex_event);

and so on.

That way frequent uses to given variable don't require the wards, there's no risk of forgetting some unlock (like lack of that 'else' statement!) and you bundle work minimizing time spent waiting in mutexes or locking/unlocking them.

Also remember: not to lose the synchronization data to cache, all inter-thread variables should be declared as volatile. Otherwise it may take a long time before your event propagates from one thread to another. But using volatile you break a lot of optimizations by the compiler. By making the non-volatile local copies you reduce the amount of work done around the volatile variables and allow the optimizer to go wild all over the copies without risk of breaking things.

SF.
  • 13,549
  • 14
  • 71
  • 107
  • Thank you. Now I am sure of which way to use. – etugcey May 10 '16 at 09:38
  • @etugcey: ...also, you're in for a nasty surprise for when your current thread overwrites the newly-written `event = 1` by the other thread, as two events are triggered in rapid sequence... but that's a subject for another question. (hint: unidirectional variables, `writer thread` and `reader thread`.) – SF. May 10 '16 at 09:56
  • @SF. event queue is natural solution for this. – Victor Sorokin May 10 '16 at 10:24
  • @SF. Could you expand on the claim that "all inter-thread variables should declared as volatile"? That's a big claim that could be misunderstood or misused, so some more explanation and support would be good. – Joe May 10 '16 at 10:27
  • @SF Please do expand on the volatile, I would like to understand more. – etugcey May 10 '16 at 10:56
  • [Here's a discussion about using volatile with threads](http://stackoverflow.com/questions/35345899/why-is-volatile-keyword-not-needed-for-thread-synchronisation). – user3386109 May 10 '16 at 11:06
  • @SF any tips on what to do to avoid situations where before you update data with data = data_copy, data gets changed by another thread, but that information is lost due to using data_copy instead of the update by the other thread? – etugcey Jun 10 '16 at 12:31
  • @etugcey: Unidirectionals. You don't update `data` in both threads. First thread only writes to `data`, the other reads it. Then the first writes conclusions/changes to, say, `data_ack` - which the second thread only reads and never writes. The second thread must deal with discrepancies then, if the `data_ack` differs from `data`. Or you hang the other thread on a mutex until the first finishes processing. (obviously undesirable if processing takes long). Or you use more complex data types: FIFOs, stacks. These still need to be unidirectional: one thread writes, the other reads. 2 dirs=2vars. – SF. Jun 10 '16 at 17:22
4

Yes, you do need synchronisation for all reads.

For the while() loop you can use this pattern:

pthread_mutex_lock(&mutex_event);
while(event) {
  pthread_mutex_unlock(&mutex_event);
  // blah blah code here
  // blah blah code here
  // blah blah code here
  pthread_mutex_lock(&mutex_event);
}
pthread_mutex_unlock(&mutex_event);

..So the lock is always held at the condition.

caf
  • 233,326
  • 40
  • 323
  • 462
3

Yes, you should always lock the mutex before reading or writing the protected variable. To avoid messy code, while still minimizing lock time, you should move the event information to a local variable that can be used after the mutex is unlocked, like this

int do_blah = 0;

pthread_mutex_lock(&mutex_event);
if(event){
    event = 0;
    do_blah = 1;
}
pthread_mutex_unlock(&mutex_event);

if ( do_blah ) {
    // blah blah code here
    // blah blah code here
    // blah blah code here
}
user3386109
  • 34,287
  • 7
  • 49
  • 68
  • Thank you for your answer. What do you think about, making locking, reading and unlocking procedure into a function, and calling the function in if statement? – etugcey May 10 '16 at 09:28
  • @etugcey just in case -- if your shared state, relevant to particular operation, consists of more than variable (as it usually does), your lock must protect whole set of variables defining shared state. Example: summing of `N` shared elements -- while summing, all of `N` must be under lock. (Well, you may unlock elements you've counted already, but it's not easy usually). – Victor Sorokin May 10 '16 at 09:51