Prevent ticket system deadlock when using DVB subtitles

When using DVB subtitles on an SMP machine, we see occasional lockups, which
appear to be caused by one thread acquiring the same ticket twice. Fix this,
by preventing acquire() and release() from blocking if the current thread has
already acquired the ticket.

Code sequences like the following can still block in all acquires and
releases:

ticket->acquire(...)
/* Do something */
ticket->release(...)

However, code sequences like the following, which used to deadlock if ticket
was revoked at just the wrong moment, now succeed:

ticket->acquire(...)
/* Do something */
ticket->acquire(...) /* This acquire cannot block */
/* Do something */
ticket->release(...) /* This release cannot block */
/* Do something */
ticket->release(...)

Without this patch, the inner acquire() and release() calls could block if
ticket was revoked at the wrong time. revoke() would not unblock the blocking
acquire until there have been as many release()s as acquire()s, which cannot
happen.

2 files changed, 66 insertions, 3 deletions

diff --git a/src/xine-engine/xine.c b/src/xine-engine/xine.c
index 254d596b1..00f1dc41c 100644
--- a/src/xine-engine/xine.c
+++ b/src/xine-engine/xine.c
@@ -127,17 +127,54 @@ void _x_extra_info_merge( extra_info_t *dst, extra_info_t *src ) {
   }
 }
 
+static int acquire_allowed_to_block(xine_ticket_t *this) {
+  pthread_t own_id = pthread_self();
+  unsigned entry;
+  unsigned new_size;
+
+  for(entry = 0; entry < this->holder_thread_count; ++entry) {
+    if(this->holder_threads[entry].holder == own_id) {
+      /* This thread may already hold this ticket */
+      this->holder_threads[entry].count++;
+      return (this->holder_threads[entry].count == 1);
+    }
+  }
+  /* If we get this far, this thread hasn't claimed this ticket before.
+     We need to give it a new entry in the list, then return true */
+  for(entry = 0; entry < this->holder_thread_count; ++entry) {
+    if(this->holder_threads[entry].count == 0) {
+      this->holder_threads[entry].holder = own_id;
+      this->holder_threads[entry].count = 1;
+      return 1;
+    }
+  }
+  /* List too small. Realloc to larger size */
+  new_size = this->holder_thread_count * 2;
+  lprintf("Reallocing from %d to %d entries\n", this->holder_thread_count, new_size);
+  
+  this->holder_threads = realloc(this->holder_threads, sizeof(*this->holder_threads) * new_size);
+  memset(this->holder_threads + this->holder_thread_count, 0, this->holder_thread_count);
+
+  /* Old size is equivalent to index of first newly allocated entry*/
+  this->holder_threads[this->holder_thread_count].count = 1;
+  this->holder_threads[this->holder_thread_count].holder = own_id;
+  this->holder_thread_count = new_size;
+
+  return 1;  
+}
+ 
 static int ticket_acquire_internal(xine_ticket_t *this, int irrevocable, int nonblocking) {
   int must_wait = 0;
 
   pthread_mutex_lock(&this->lock);
+  int allowed_to_block = acquire_allowed_to_block(this);
   
   if (this->ticket_revoked && !this->irrevocable_tickets)
     must_wait = !nonblocking;
   else if (this->atomic_revoke && !pthread_equal(this->atomic_revoker_thread, pthread_self()))
     must_wait = 1;
 
-  if (must_wait) {
+  if (must_wait && allowed_to_block) {
     if (nonblocking) {
       pthread_mutex_unlock(&this->lock);
       return 0;
@@ -162,9 +199,25 @@ static void ticket_acquire(xine_ticket_t *this, int irrevocable) {
   ticket_acquire_internal(this, irrevocable, 0);
 }
 
+static int release_allowed_to_block(xine_ticket_t *this) {
+  pthread_t own_id = pthread_self();
+  unsigned entry;
+  
+  for(entry = 0; entry < this->holder_thread_count; ++entry) {
+    if(this->holder_threads[entry].holder == own_id) {
+      this->holder_threads[entry].count--;
+      return this->holder_threads[entry].count == 0;
+    }
+  }
+  lprintf("BUG! Ticket 0x%p released by a thread that never took it! Allowing code to continue\n", this);
+  _x_assert(0);
+  return 1;
+}
+
 static void ticket_release_internal(xine_ticket_t *this, int irrevocable, int nonblocking) {
 
   pthread_mutex_lock(&this->lock);
+  int allowed_to_block = release_allowed_to_block(this);
   
   this->tickets_granted--;
   if (irrevocable)
@@ -172,8 +225,10 @@ static void ticket_release_internal(xine_ticket_t *this, int irrevocable, int no
   
   if (this->ticket_revoked && !this->tickets_granted)
     pthread_cond_broadcast(&this->revoked);
-  if (this->ticket_revoked && !this->irrevocable_tickets && !nonblocking)
-    pthread_cond_wait(&this->issued, &this->lock);
+  if (allowed_to_block) {
+    if (this->ticket_revoked && !this->irrevocable_tickets && !nonblocking)
+      pthread_cond_wait(&this->issued, &this->lock);
+  }
   
   pthread_mutex_unlock(&this->lock);
 }
@@ -262,6 +317,8 @@ static xine_ticket_t *ticket_init(void) {
   port_ticket->issue                = ticket_issue;
   port_ticket->revoke               = ticket_revoke;
   port_ticket->dispose              = ticket_dispose;
+  port_ticket->holder_thread_count = XINE_MAX_TICKET_HOLDER_THREADS;
+  port_ticket->holder_threads = calloc(XINE_MAX_TICKET_HOLDER_THREADS,sizeof(*port_ticket->holder_threads));
   
   pthread_mutex_init(&port_ticket->lock, NULL);
   pthread_mutex_init(&port_ticket->revoke_lock, NULL);
diff --git a/src/xine-engine/xine_internal.h b/src/xine-engine/xine_internal.h
index 5f9a82f97..5523001ca 100644
--- a/src/xine-engine/xine_internal.h
+++ b/src/xine-engine/xine_internal.h
@@ -75,6 +75,7 @@ extern "C" {
 
 #define XINE_MAX_EVENT_LISTENERS         50
 #define XINE_MAX_EVENT_TYPES             100
+#define XINE_MAX_TICKET_HOLDER_THREADS   64
 
 /* used by plugin loader */
 #define XINE_VERSION_CODE                XINE_MAJOR_VERSION*10000+XINE_MINOR_VERSION*100+XINE_SUB_VERSION
@@ -179,6 +180,11 @@ struct xine_ticket_s {
   int             pending_revocations;
   int             atomic_revoke;
   pthread_t       atomic_revoker_thread;
+  struct {
+    int count;
+    pthread_t holder;
+  } *holder_threads;
+  unsigned        holder_thread_count;
 #endif
 };