Avoiding deadlock with mutex trylock in multithreaded app.

[Pierre Mallard <mallard dot pierre at gmail dot com>]

Hi,

Sorry for this long mail, and thanks for your help and advice :

I have a question regarding proper handling of non concurrent access
between two (or more) thread that deals together on a
register/unregister + callback way.

Let's try to be clear !

A Threaded Module "A" acts like this :
It offers two function to a module B and has a main task :
 "A_register" register request, called by module B in a Start
function, called by a thread "C"
 "A_unregister" unregister request , called by module B in a Release
function, in a thread "C"
 "A_process" a thread main task that loop forever and unqueue request,
call multiple time the callback that comes with the request untill
request processing is finished and start again

A module B that is driven by a task C that has three function :
"B_start" register to A
"B_callback" calls by A during request processing
"B_release" unregister to A, and destroy B

I need to avoid B_callback to be called during and after B_release

I can think of two way things could work :
   A "deadlock way" that leads to a deadlock (!),
   A "Trylock way" that requires trylock

I present here both approach and attached a code sample to illustrate
the trylock way I can think of.

I am wondering whether the trylock approach is the good one, and also
how could I avoid the waste of time (and CPU !) trying to lock mutex_B
?

************************************************************
"The deadlock way"
A Module A mutex between callback calls and unregister function, to
check if request is still existing,
A Module B mutex between release and callback.
But that leads to a dead lock :

=====================================
Thread A
=====================================
"A_process" lock mutex_A to check whether request is still registered
if !stop_cur_request
  B_Callback lock/unlock mutex_B
unlock mutex_A

=====================================
Thread C
=====================================
"B_release" lock mutex_B
  "A_Unregister" lock/unlock mutex A
unlock mutex_B

The dead lock appears because in task A we lock mutex_A then mutex_B
and in task C we lock mutex_B then mutex_A.

***********************************************************
"The TryLock way"
I can Trylock instead of lock in Callback, Therefore, when release is
called and lock mutex_B, callback won't hold mutex_A if it realize
mutex_B is busy.

=====================================
Thread A
=====================================
"A_process"
...
while (TRUE)
      lock mutex_A
      if !stop_cur_request
      	 if Call Callback == RETRY <-- TryLock Failed
	    unlock mutex_A
      	 else
	    break
	 endif
      endif
endWhile
unlock mutex_A
...


Thanks a lot for your attention, comments and advice,
Regards,
Pierre


-- 
Pierre Mallard

#include <pthread.h>
#include <unistd.h>
#include <stdio.h>

typedef int (*callback_t)(void *);

#define NB_B 10
#define A_QUEUE_SIZE (NB_B + 1) //+1 for circular

/* request structure of Module A */
struct a_req{
  callback_t func;
  void * param;
};

/* Module A */
struct a {
  pthread_t task;
  pthread_mutex_t mutex;
  struct a_req queue[A_QUEUE_SIZE];
  int rd;
  int wr;

  struct a_req * cur_req;
};

void * a_task(void * param)
{
  struct a * this = (struct a *) param;
  while (1){
    pthread_testcancel();

    //get a request
    pthread_mutex_lock(&this->mutex);
    if (this->rd != this->wr){
      this->cur_req = &this->queue[this->rd];
      this->rd = (this->rd ++)%A_QUEUE_SIZE;
    }
    else {
      pthread_mutex_unlock(&this->mutex);
      continue;
    }
    fprintf(stderr,"New B : %p\n",this->cur_req->param);
    pthread_mutex_unlock(&this->mutex);

    //Call callback till unregister
    while (1){
      pthread_mutex_lock(&this->mutex);
      if (this->cur_req == NULL){
	pthread_mutex_unlock(&this->mutex);
	break;
      }
      if (this->cur_req->func(this->cur_req->param) == 1){
	fprintf(stderr,"TryLocked refused\n");
	pthread_mutex_unlock(&this->mutex);
	continue;
      }
      pthread_mutex_unlock(&this->mutex);
    }
  }
  pthread_exit(NULL);
}

int a_init(struct a * this)
{
  this->rd = this->wr;
  pthread_mutex_init(&this->mutex,NULL);
  this->cur_req = NULL;
  if (pthread_create(&this->task,NULL,&a_task,this) < 0){
    return -1;
  }
  return 0;
}

void a_release(struct a * this){
  if (pthread_cancel(this->task) < 0){
    // well that's annoying ... !
    return;
  }
  pthread_join(this->task,NULL);
  pthread_mutex_destroy(&this->mutex);
  return;
}

void * a_register(struct a * this,callback_t func, void * param){
  void * ret;
  pthread_mutex_lock(&this->mutex);
  if (this->wr == (this->rd-1) % A_QUEUE_SIZE){
    pthread_mutex_unlock(&this->mutex);
    return NULL;
  }
  this->queue[this->wr].func = func;
  this->queue[this->wr].param = param;
  ret = &this->queue[this->wr];
  this->wr = (this->wr + 1) % A_QUEUE_SIZE;
  pthread_mutex_unlock(&this->mutex);
  return ret;
}

int a_unregister(struct a * this,void * req){
  pthread_mutex_lock(&this->mutex);
  if (req != this->cur_req){ // we do not handle still pending request ...
    pthread_mutex_unlock(&this->mutex);
    return -1;
  }
  this->cur_req = NULL;
  pthread_mutex_unlock(&this->mutex);
  return 0;
}

struct b{
  pthread_mutex_t mutex;
  void * req;
  struct a * a;
};

int b_init(struct b * this,struct a * a)
{
  this->req = NULL;
  this->a = a;
  return pthread_mutex_init(&this->mutex,NULL);
}

int b_callback(void * vthis){
  struct b * this = (struct b *) vthis;
  if (pthread_mutex_trylock(&this->mutex) < 0){
    return 1;
  }
  //do sthg...
  pthread_mutex_unlock(&this->mutex);
  return 0;
}

int b_start(struct b * this){
  if ((this->req = a_register(this->a,&b_callback,this)) == NULL){
    return -1;
  }
  return 0;
}

void b_release(struct b * this){
  pthread_mutex_lock(&this->mutex);
  if (this->req != NULL){
    a_unregister(this->a,this->req);
  }
  sleep(4);
  pthread_mutex_unlock(&this->mutex);
  pthread_mutex_destroy(&this->mutex);
}

int main(void){
  struct a a;
  struct b b[NB_B];
  int i;

  if (a_init(&a) < 0){
    return -1;
  }

  if (b_init(&b[i],&a) < 0){
    return -1;
  }

  for (i = 0 ; i < NB_B; i++){
    b_init(&b[i],&a);
    b_start(&b[i]);
  }

  for (i = 0 ; i < NB_B; i++){
    sleep(10);
    fprintf(stderr,"Releasing %p\n",&b[i]);
    b_release(&b[i]); /* b_stop is performed by b_release */
  }
  a_release(&a);
  
  return 0;
}