Mutex Deadlock (pthread_mutex)

Question

I can't understand why in this code I get a deadlock. I have defined this mutexes:

mutex3: = 0 (LOCKED)
mutex2: = 1 (UNLOCKED)

I have 2 processes: Father and son. Each has N threads that I pass by argument. The child's threads contend with the father's threads a "resource" (typical producer / consumer problem).

Thanks in advance!

I suppose the problem is here:

void * func_padre(void * n)
{
    while(1)
    {
        pthread_mutex_lock(&mutex2);
        printf("Scrivi messaggio: ");
        fflush(stdout);
        scanf("%[^\n]",(char *)addr_mem);
        getchar();
        pthread_mutex_unlock(&mutex3);

    }
}

void * func_figlio(void * n)
{
    while(1)
    {   
        pthread_mutex_lock(&mutex3);
        write(fd,addr_mem,4096);
        lseek(fd,0,SEEK_SET);
        memset(addr_mem,0,4096);
        pthread_mutex_unlock(&mutex2);
    }
}

CODE:

#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <pthread.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/ipc.h>
#include <sys/shm.h>
#include <string.h>

void * addr_mem;
pthread_mutex_t mutex2,mutex3;
int fd;
void * func_padre(void * n)
{
    while(1)
    {
        pthread_mutex_lock(&mutex2);
        printf("Scrivi messaggio: ");
        fflush(stdout);
        scanf("%[^\n]",(char *)addr_mem);
        getchar();
        pthread_mutex_unlock(&mutex3);

    }
}

void * func_figlio(void * n)
{
    while(1)
    {   
        pthread_mutex_lock(&mutex3);
        write(fd,addr_mem,4096);
        lseek(fd,0,SEEK_SET);
        memset(addr_mem,0,4096);
        pthread_mutex_unlock(&mutex2);
    }
}

int main(int argc, char*argv[])
{
        if(argc<2)
        {
            printf("POCHI PARAMETRI\n");
            fflush(stdout);
            exit(-1);
        }
        
        int val=strtol(argv[2],0,10);

        fd = open(argv[1],O_CREAT|O_RDWR,0666);
        lseek(fd,0,SEEK_SET);//USELESS
        
        ///SHARED MEMORY
        int id_mem;

        
        id_mem = shmget(6543,4096,IPC_CREAT|0666);
        addr_mem = shmat(id_mem,NULL,0);
        /////////////////////
        
        /// MUTEX
        pthread_mutex_init(&mutex2,NULL);
        pthread_mutex_init(&mutex3,NULL);
        pthread_mutex_lock(&mutex3);
        /////////////////////
        pthread_t tid;
        int i=0;
            
        int pid;
        pid = fork();
        if(pid==0)
        {
            //CHILD
            for(i=0; i<val; i++)
                pthread_create(&tid,NULL,func_figlio,NULL);
                
            while(1)
            {   
                pthread_mutex_lock(&mutex3);
                write(fd,addr_mem,4096);
                memset(addr_mem,0,4096);
                pthread_mutex_unlock(&mutex2);
            }
        }
        else
        {
            //FATHER
            for(i=0; i<val; i++)
                pthread_create(&tid,NULL,func_padre,NULL);
                
                while(1)
                {
                    pthread_mutex_lock(&mutex2);
                    printf("Scrivi messaggio: ");
                    fflush(stdout);
                    scanf("%[^\n]",(char *)addr_mem);
                    getchar();
                    pthread_mutex_unlock(&mutex3);
                    
                }
        }
}

regarding: `pthread_mutex_t mutex2,mutex3;` the code should separate this into two statements. Where each declaration of a mutex has an initializer, similar to: `pthread_mutex_t mutex2 = PTHREAD_MUTEX_INITIALIZER;` — user3629249, Jun 24 '19 at 21:17
OT: regarding: `pthread_create(&tid,NULL,func_padre,NULL);` Always check the returned value. Any value other than 0 indicates the function failed. When it fails, then call `perror( "pthread_create failed" ); — user3629249, Jun 24 '19 at 21:19
OT: immediately after: `pid = fork();` should be: `if( pid < 0 ) { perror( "fork failed" ); exit( EXIT_FAILURE ); }` — user3629249, Jun 24 '19 at 21:21
OT: regarding: `printf("POCHI PARAMETRI\n"); fflush(stdout);` 1) an error message should be output to `stderr`, not `stdout` 2) a USAGE message should be output, telling the user what they should have done. Similar to: `fprintf( stderr, "USAGE: %s \n", argv[0] );` — user3629249, Jun 24 '19 at 21:25
regarding: `fd = open(argv[1],O_CREAT|O_RDWR,0666);` 1) the `O_CREAT` indicates the file must not have already existed, so `O_RDWR` is not a good choice as cannot read from an file that contains nothing. 2) always check the returned value to be >=0. If less than 0 then call: `perror( "open failed" );` followed by: `exit( EXIT_FAILURE );` — user3629249, Jun 24 '19 at 21:28
OT: regarding: `write(fd,addr_mem,4096);` Always check the returned value (it has type `ssize_t` ) the returned value can be less than 4096 if any error occurred, like the disk is full, It can be <0 if a major error occurred, then call: `perror( "write failed" );` to output both your error message and the text reason the system thinks the error occurred — user3629249, Jun 24 '19 at 21:33
OT: regarding: `id_mem = shmget(6543,4096,IPC_CREAT|0666);` and `addr_mem = shmat(id_mem,NULL,0);` Both of these statements can fail. Always check the returned value, and if `-1` then call: `perror( "you error message" ); cleanup, then exit the program — user3629249, Jun 24 '19 at 21:36
thank you very much for the tricks. This code was written more to understand why the mutexes didn't work than to actually be used. Thank you again for the granular precision of the answers. — ABC, Jun 24 '19 at 21:53

score 1 · Answer 1 · answered Jun 24 '19 at 00:47

1

Your problem is not a deadlock, it's just buggy code. A thread cannot unlock a mutex it did not lock. Your while loops all unlock mutexes they do not hold and never unlock the mutexes they do hold.

answered Jun 24 '19 at 00:47

David Schwartz

179,497
17
214
278

if I wanted to synchronize a situation like this with mutexes what should I do? – ABC Jun 24 '19 at 06:43
am I forced to use semaphores "System V"? – ABC Jun 24 '19 at 08:40
No, just fix the bugs. – David Schwartz Jun 25 '19 at 00:35

score 0 · Answer 2 · answered Jun 24 '19 at 21:13

suggest:

have global variable that is cycled through a limited set of values, say: 0, 1, 0 ...

Use the mutex to stop other threads from progressing, I.E.

When a call (in thread) returns from locking the mutex, 
then check 
    if the value in the global variable is for 'this' thread.
    then  
        process the thread activities,  
        increment the global variable,
    endif 
endif
unlock the mutex
call nanosleep() to delay for a while to allow other threads to run

Mutex Deadlock (pthread_mutex)

2 Answers2