Skip navigation.
Home
The QNX Community Portal

View topic - Do I need to lock a mutex for reading a variable?

Do I need to lock a mutex for reading a variable?

For discussion of realtime and/or embedded programming.

Do I need to lock a mutex for reading a variable?

Postby grillosolitario » Tue Oct 27, 2009 6:48 pm

Hello,

This might be very obvious, and I guess the answer is no. But I must be sure.

Let's suppose I have several threads that use a global variable. If I want to change its value, I would use a mutex. But if a thread just wants to read the value, is it mandatory to lock the mutex?

Code: Select all
 int i;

 thrd1:
 pthread_mutex_lock(&mutex);
 i++;
 pthread_mutex_unlock(&mutex);

 thrd2:
int j;
...
[b] pthread_mutex_lock(&mutex);[/b]
 j=i;
[b] pthread_mutex_unlock(&mutex);[/b]


I know the code is correct, but could I do:

thrd1:
pthread_mutex_lock(&mutex);
i++;
pthread_mutex_unlock(&mutex);

thrd2:
int j;
...
// No mutex_lock
j=i;
// No mutex_unlock

Thanks in advance.
grillosolitario
Active Member
 
Posts: 91
Joined: Mon Oct 18, 2004 4:47 pm
Location: Spain

Postby Tim » Tue Oct 27, 2009 7:36 pm

Technically the answer is 'no' but in reality you should lock the mutex to read the value.

One reason is that the statement 'j=i' looks like it's a simple atomic assignment. But in reality plenty of underlying assembly/hex instructions get generated to perform this assignment. So it's possible that the value could change in the middle of executing all those low level instructions. Depending on how the real code is implemented that could cause all kinds of bugs. For example if your code was:

Code: Select all
j = i;
for (a=0; a<j; a++)
{
// Walk some linked list or index into some array etc
}


You could never be sure the for loop would evaluate correctly unless you used the mutex. You could walk off the end of a partially constructed linked list or index outside an array that was being resized etc.

Also if you aren't locking the mutex when you are reading 'i' in another thread then what is the point of having the mutex in the first place (ie what are you using this global variable for because maybe you don't need a mutex at all)? Because if you are saying you can read 'i' without needing the mutex locked then you are basically saying you can write 'i' without needing it locked as well.

Plus from a code maintenance point of view is that without the mutex someone else coming along won't know that 'i' is a shared variable between threads.

Tim
Tim
Senior Member
 
Posts: 1388
Joined: Wed Mar 10, 2004 12:28 am

Postby grillosolitario » Wed Oct 28, 2009 4:05 pm

Hello,

I posted an answer yesterday, but it doesn't appear.

First of all thank you very much for your answer.

LetÅ› see if I've understood it correctly: it's safe not to use the mutex, but it could lead to a program misfunction, isn't it?
grillosolitario
Active Member
 
Posts: 91
Joined: Mon Oct 18, 2004 4:47 pm
Location: Spain

Postby Tim » Wed Oct 28, 2009 6:08 pm

It's NOT safe if you don't lock the mutex on read access. It's merely legal from a coding standpoint to do so.

What you should instead be asking is whether you need a mutex in your design. If you decide you need a mutex then you should be using it for read and write access.

I've personally never seen a design where you need a mutex for write access and NOT for read access. It doesn't make logical sense.

Tim
Tim
Senior Member
 
Posts: 1388
Joined: Wed Mar 10, 2004 12:28 am

Postby grillosolitario » Wed Oct 28, 2009 6:21 pm

Ok. Thank you very much, Tim.
grillosolitario
Active Member
 
Posts: 91
Joined: Mon Oct 18, 2004 4:47 pm
Location: Spain

Postby mario » Wed Oct 28, 2009 6:53 pm

If applicable you can use the atomic_* set of functions to modify the variable, then you wouldn't need a mutex to lock read operation.
mario
QNX Master
 
Posts: 4132
Joined: Sun Sep 01, 2002 1:04 am

Postby Tim » Thu Oct 29, 2009 6:45 pm

One last thing. If you are worried about polluting your code with lots of lock/unlock calls for a mutex every time you read/write to a global variable there are some things you can do to:

A) Improve readability
B) Reduce the odds that someone forgets to lock/unlock a mutex

If you are using C++ here is a very simple template class (along with sample use) that does all the mutex management work for you. This assumes your global variables are just simple built in types like int, float etc.

Code: Select all
#include <errno.h>
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <unistd.h>
#include <sys/neutrino.h>
#include <sys/syspage.h>
#include <inttypes.h>

using namespace std;

// Global variables
int foo = 10;
float foo1 = 10.5;

template <class T>
class GlobalVariable
{
   public:
        GlobalVariable(T &global) :
          mGlobal(global)
        {
         pthread_mutexattr_t mutexAttr;   // Mutex attributes
         int retVal = pthread_mutexattr_init(&mutexAttr);
         if (retVal != EOK)
         {
            printf("Error %s while trying to initialize mutex attributes!", strerror(retVal));
         }
         
         // Set the mutex to allow recursive calls
         retVal = pthread_mutexattr_setrecursive(&mutexAttr, PTHREAD_RECURSIVE_ENABLE);
         if (retVal != EOK)
         {
            printf("Error %s while trying to set mutex attributes!", strerror(retVal));
         }
         
         // Initialize the mutex to an unlocked state
         retVal = pthread_mutex_init(&mMutex, &mutexAttr);
         if (retVal != EOK)
         {
            printf("Error %s while trying initialize mutex!", strerror(retVal));
         }
        };
   
      virtual ~GlobalVariable(void)
        {
         pthread_mutex_destroy(&mMutex);
      };
 
        // Assignment operators to/from the global variable
        operator int()
        {
         pthread_mutex_lock(&mMutex);
         printf ("Read from the global-int\n");
         int retVal = (int)mGlobal;
         pthread_mutex_unlock(&mMutex);
         
         return retVal;
      };
        operator float()
        {
         pthread_mutex_lock(&mMutex);
         printf ("Read from the global-float\n");
         float retVal = (float)mGlobal;
         pthread_mutex_unlock(&mMutex);
         
         return retVal;
      };   
        GlobalVariable& operator =(const T variable)
        {
         pthread_mutex_lock(&mMutex);
         printf ("Assign to the global\n");
         mGlobal = variable;
         pthread_mutex_unlock(&mMutex);
         
         return *this;
      };
   
   private:
   
       T &mGlobal;                       // Reference to the global variable
        pthread_mutex_t mMutex;           // Mutex
   
        // Explicitly disallow the default constructor because the mGlobal
      // reference variable must be set in the constructor.
      GlobalVariable();
   
        // Explicitly disallow copy operator because mutexes are created in
      // this class.
        GlobalVariable(const GlobalVariable &orig);
};

// The GNU GCC 3.3.5 compiler does not support the 'export' keyword so all
// function templates must be available in every translation unit (Ie source
// file) and hence these must go in the header file along with the
// GlobalVariable class definition.
template GlobalVariable<int>;
template GlobalVariable<float>;

GlobalVariable<int> bar(foo);      // One reference to global int foo
GlobalVariable<float> bar1(foo1);  // One reference to global float foo1

void main(void)
{
   // Read the initial global variables (directly and via the class)
   printf ("foo is %d and bar is %d initially\n", foo, (int)bar);
   printf ("foo1 is %f and bar1 is %f initially\n", foo1, (float)bar1);

   // Assign to the global variables
   printf("Change the globals\n");
   bar = 5;
   bar1 = 5.5;
   
   // Example of use in an array.
   printf("Loop 5 times\n");
   for (int i=0; i<(int)bar; i++)
   {
      printf("%d\n", i);
   }

   // Read the final global variables (directly and via the class)
    printf ("Finish up\n");
   printf ("foo is %d and bar is %d at program end\n", foo, (int)bar);
   printf ("foo1 is %f and bar1 is %f at program end\n", foo1, (float)bar1);
}


If you are using C you can't quite get the same elegant solution. Instead you'd be forced to create two Macros per global variable (one to set, one to read) and the Macros would manage the lock/unlocking.

Tim
Tim
Senior Member
 
Posts: 1388
Joined: Wed Mar 10, 2004 12:28 am

Postby mario » Thu Oct 29, 2009 8:23 pm

A simple int like this can be set to volatile and that should be good enough ;-)
mario
QNX Master
 
Posts: 4132
Joined: Sun Sep 01, 2002 1:04 am

Postby Tim » Fri Oct 30, 2009 4:54 pm

Mario,

Very true. Which is why I questioned the original design and whether it needed a mutex at all.

I merely provided the C++ class because I was bored on my lunch hour having spent most of the past couple weeks at work updating some design documents and writing some manuals. So I needed an excuse to write some code and decided to see if I could whip up a quick template class in under an hour :-)

Tim
Tim
Senior Member
 
Posts: 1388
Joined: Wed Mar 10, 2004 12:28 am

Postby mushtaqk » Fri Mar 26, 2010 2:52 pm

Hi,

This is regarding creating Recursive mutex. Iam not able to enable recursive in the mutex attribute.I tried with the following code to set the attribute to PTHREAD_RECURSIVE_ENABLE. But still the attribute is PTHREAD_RECURSIVE_DISABLE read through pthread_mutexattr_getrecursive.

if (pthread_mutexattr_setrecursive(&attr,PTHREAD_RECURSIVE_ENABLE) == EOK) {
if (pthread_mutexattr_getrecursive(&mutex,&rec) == EOK) {
printf("Recursive %d",rec);
}
}
The Value of rec varibale is 0 (PTHREAD_RECURSIVE_DISABLE). Even tried setting attribute directly " attr.__flags = PTHREAD_RECURSIVE_ENABLE;", still the value returned by the pthread_mutexattr_getrecursive is 0. QNX Version is 4.5.0.
Please let me know, how i can set the attribute to Recursive.

Sorry iam posting my query here, i couldn't start the new topic.
Thanks,
-Mushtaq Khan
mushtaqk
New Member
 
Posts: 2
Joined: Fri Mar 26, 2010 2:26 pm


Return to Realtime and Embedded

Who is online

Users browsing this forum: No registered users and 2 guests