Is it right to use mutex in the way used in this program?

Question

I am new to C++, and I have started doing multi-threading in C++. Can you please comment on the following program? Is it a right way of using mutex? The other question is to identify the shared resource in C++ is easy - just look at static members. C++ doesn't have the concept of global variables, and therefore we can just look at the static members of a class? Afterwards, decide what should be serialized - mutex locked/unlocked? In C, it is a bit challenge to identify the shared resources, as there is a concept of global variables. Can you correct me on my understanding?

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <iostream>
/** get pid **/
#include <sys/types.h>
#include <unistd.h>

using namespace std;


class helium_thread
{
  private:
  pthread_t *thread_id;

  public:
  static pthread_mutex_t mutex_thread;
  void set_thread_id(pthread_t tid);
  pthread_t *get_thread_id();
  int create_thread(pthread_t *thread_ptr, const pthread_attr_t *attr, void * (*start_routine)(void *), void *arg );
  helium_thread();  
  ~helium_thread();

};

void helium_thread::set_thread_id( pthread_t tid)
{
   *(this->thread_id) = tid;    
}

pthread_t * helium_thread::get_thread_id( )
{
   return (this->thread_id);
}

int helium_thread::create_thread(pthread_t *thread_ptr, const pthread_attr_t *attr, void * (*start_routine)(void *), void *arg )
{
   int ret;
   ret = pthread_create(thread_ptr,attr,start_routine,(void *)arg)  ;
   cout<<"Thread created "<<std::hex<<thread_ptr<<endl;
   return ret;

}

helium_thread::helium_thread()
{

    thread_id = new pthread_t;
    cout<<"Constructor called "<<std::hex<<thread_id<<endl;
}

helium_thread::~helium_thread()
{
    cout<<"Destructor called"<<std::hex<<thread_id<<endl;
    delete thread_id;
}

/** While defining the methods of the class, Keywords static and virtual should not be repeated in the definition. **/
/** They should only be used in the class declaration. **/

void *Thread_Function(void *thread_arg)
{
  pthread_mutex_lock(&(helium_thread::mutex_thread));   
  cout<<"Inside Thread_Function"<<endl; 
  pid_t *thread_pid_val  = (pid_t *) thread_arg;
  /**  std::hex will print the value isn hexadecimal **/
  cout<<"The process pid is "<< std::hex << (*thread_pid_val) <<endl;
  pthread_t ptid = pthread_self();
  cout<<" The thread id is " << std::hex<< ptid << endl;
  pthread_mutex_unlock(&(helium_thread::mutex_thread)); 

}

/** The definition of the static member can't be inside a function, You need to put it outside **/
/** When I tried using inside a function, I got the error - error: invalid use of qualified-name ‘helium_thread::mutex_thread **/

pthread_mutex_t helium_thread::mutex_thread = PTHREAD_MUTEX_INITIALIZER;

int main(int argc, char *argv[])
{
   helium_thread thread_1, thread_2;
   pid_t thread_pid_val = getpid();
   pthread_t thread_1_id;

   thread_1.create_thread((thread_1.get_thread_id()),NULL,Thread_Function,&thread_pid_val);
   thread_2.create_thread((thread_2.get_thread_id()),NULL,Thread_Function,&thread_pid_val);
   pthread_join( *(thread_1.get_thread_id()), NULL);
   pthread_join( *(thread_2.get_thread_id()), NULL);

   return  0;   
}

Answer 1

A few things:

1. Globals can be used in C++ but try to avoid them, especially in multithreaded code.
2. You didn't initialize or destroy the mutex anywhere.
3. In C++ you should not call mutex_lock/unlock directly but have a wrapper class do it for you, to avoid keeping the mutex locked on an early return from the function (return on error or exception).

Example:

class auto_lock {
    pthread_mutex_t* mutex;
public:
    auto_lock(pthread_mutex_t* _mutex) : mutex(_mutex) {
        if (mutex) pthread_mutex_lock(mutex);
    }
    ~auto_lock(){
        if (mutex) pthread_mutex_unlock(mutex);
    }
};