[mythtv-commits] Ticket #7528: Backend Deadlock during rescheduling

[MythTV]

#7528: Backend Deadlock during rescheduling
-----------------------------------------------+----------------------------
 Reporter:  Jelle Foks <jelle-mythtv@…>        |       Owner:  gigem  
     Type:  defect                             |      Status:  new    
 Priority:  minor                              |   Milestone:  unknown
Component:  MythTV - Scheduling                |     Version:  unknown
 Severity:  medium                             |     Mlocked:  0      
-----------------------------------------------+----------------------------
 I've had deadlocks in the backend that made the backend appear completely
 unresponsive to frontends (can't get a list of recorded shows, can't start
 playback of a recording etc). The only way out was restarting mythbackend
 on the master backend.

 After experiencing the deadlock regularly, more than just a couple of
 times, I saw that it seemed to occur during re-scheduling.

 Examining the use of mutexes around mythtv made me dizzy, there are so
 many different ones and it wasn't very clear to me which locks are or
 could be held during which functions...

 But I found the instance below that looked like a potential source of the
 deadlocks I had been encountering, because it would re-acquire reschedLock
 before releasing recordmatchLock, so the thread may be blocked while
 holding that lock, which would be a deadlock if another thread is trying
 to get recordmatchLock while holding reschedLock. And since I'm not sure
 if it's guaranteed by the C++ standard or g++ compiler that the
 QMutexLocker constructor is not run before the reschedLock.unlock()
 statement above it, that may be the case right there in the same
 function...

 So I made the change below in my mythtv to make sure it wasn't a deadlock
 anymore.

 I haven't seen the backend deadlocks anymore since applying the change
 below (tested for months now, while before the patch I would sometimes get
 the deadlock more than once in a week).

 Maybe not the cleanest way to do it, but it worked for me.

 Looking at the other call to UpdateMatches and the other use of
 recordmatchLock, the use of the lock doesn't make sense to me anyway (it
 makes one call to a mysql exec exclusive to a call to UpdateMatches
 elsewhere, but doesn't do that for all mysql exec calls not all
 UpdateMatches calls, so I'm at a loss at what the mutex is trying to
 achieve).

 Perhaps it really should be called UpdateMatchesLock and be locked only
 inside of UpdateMatches... Or perhaps the whole locking logic around the
 scheduler should be revisited?

 {{{
 --- mythtv/programs/mythbackend/scheduler.cpp   (revision 22704)
 +++ mythtv/programs/mythbackend/scheduler.cpp   (working copy)
 @@ -1610,8 +1610,10 @@
                          reschedQueue.clear();

                      reschedLock.unlock();
 -                    QMutexLocker locker(&recordmatchLock);
 -                    UpdateMatches(recordid);
 +                    {
 +                        QMutexLocker locker(&recordmatchLock);
 +                        UpdateMatches(recordid);
 +                    }
                      reschedLock.lock();
                  }
              }
 }}}

-- 
Ticket URL: <http://svn.mythtv.org/trac/ticket/7528>
MythTV <http://www.mythtv.org/>
MythTV