CODEWITHSUNDEEP
c++linuxmultithreadingpthreads
opc0de
opc0de

Reputation: 11768

Multiple threads and mutexes

I am very new to Linux programming so bear with me. I have 2 thread type that perform different operations so I want each one to have it's own mutex. Here is the code I am using , is it good ? If not why ?

static pthread_mutex_t cs_mutex = PTHREAD_MUTEX_INITIALIZER;
static pthread_mutex_t cs_mutex2 = PTHREAD_MUTEX_INITALIZER;

void * Thread1(void * lp)
{
    int * sock = (int*)lp;
    char buffer[2024];

    int bytecount = recv(*sock, buffer, 2048, 0);
    while (0 == 0)
    {
        if ((bytecount ==0) || (bytecount == -1))
        {
        pthread_mutex_lock(&cs_mutex);

                  //Some uninteresting operations witch plays with set 1 of global variables;
        pthread_mutex_unlock(&cs_mutex);
        }
    }
}

void * Thread2(void * lp)
{   
    while (0 == 0)
    {
        pthread_mutex_lock(&cs_mutex2);
            //Some uninteresting operations witch plays with some global variables;
        pthread_mutex_unlock(&cs_mutex2);
    }
}

Upvotes: 0

Views: 9049

Answers (5)

djna
djna

Reputation: 55957

I see three problems here. There's a question your infinite loop, another about your intention in having multiple threads, and there's a future maintainability "gotcha" lurking.

First

int bytecount = recv(*sock, buffer, 2048, 0);
while (0 == 0)

Is that right? You read some stuff from a socket, and start an infinite loop without ever closing the socket? I can only assume that you do some more reading in the loop, but in which case you are waiting for an external event while holding the mutex. In general that's a bad pattern limiting your concurrency. A possibly pattern is to have one thread reading the data and then passing the read data to other threads which do the processing.

Next, you have two different sets of resources each protected by their own mutex. You then intend to have a set of Threads for each resource. But each thread has the pattern

   take mutex
       lots of processing
   release mutex
       tiny window (a few machine instructions)
   take mutex again
        lots of processing
   release mutex
        next tiny window

There's virtually no opportunity for two threads to work in parallel. I question whether your have need for multiple threads for each resource.

Last there's a potential maintenance issue. I'm just pointing this out for future reference, I don't think you need to do anything right now. You have two functions, intended for use by two threads, but in the end they are just functions that can be called by anyone. If later maintenance results in those functions (or refactored subsets of the functions) then you could get two threads

    take mutex 1
    take mutex 2

and the other

    take mutex 2
    take mutex 1

Bingo: deadlock.

Not an easy problem to avoid, but at the very least one can aid the maintainer by careful naming choices and refactoring.

Upvotes: 3

David Schwartz
David Schwartz

Reputation: 182885

If you only have one driver, there is no advantage to having two cars. Your Thread2 code can only make useful progress while holding cs_mutex2. So there's no point to having more than one thread running that code. Only one thread can hold the mutex at a time, and the other thread can do no useful work.

So all you'll accomplish is that occasionally the thread that doesn't hold the mutex will try to run and have to wait for the other. And occasionally the thread that does hold the mutex will try to release and re-acquire it and get pre-empted by the other.

This is a completely pointless use of threads.

Upvotes: 6

viktorgt
viktorgt

Reputation: 273

Normally, a mutex is not thread related. It ensures that a critical area is only accessed by a single thread. So if u have some shared areas, like processing the same array by multiple threads, then you must ensure exclusive access for this area. That means, you do not need a mutex for each thread. You need a mutex for the critical area.

Upvotes: 7

Jim Diamond
Jim Diamond

Reputation: 1274

If you have 2 different sets of data and 2 different threads working on these sets -- why do you need mutexes at all? Usually, mutexes are used when you deal with some shared piece of data and you don't want two threads to deal with it simultaneously, so you lock it with mutex, do some stuff, unlock.

Upvotes: 2

nogard
nogard

Reputation: 9716

I think your code is correct, however please note 2 things:

  1. It is not exception safe. If exception is thrown from Some uninteresting operations then your mutex will be never unlocked -> deadlock

  2. You could also consider using std::mutex or boost::mutex instead of raw mutexes. For mutex locking it's better to use boost::mutex::scoped_lock (or std:: analog with modern compiler)

    void test()
{
    // not synch code here
    {
        boost::mutex::scoped_lock lock(mutex_);
        // synchronized code here
    }
}

Upvotes: 2

Related Questions