[mythtv-commits] Ticket #6305: Incorrect QWaitCondition & mutex usage in multiple places

MythTV mythtv at cvs.mythtv.org
Wed Mar 11 17:00:45 UTC 2009


#6305: Incorrect QWaitCondition & mutex usage in multiple places
---------------------------------------+------------------------------------
 Reporter:  tomimo at ncircle.nullnet.fi  |        Owner:  ijr    
     Type:  defect                     |       Status:  closed 
 Priority:  minor                      |    Milestone:  unknown
Component:  MythTV - General           |      Version:  head   
 Severity:  medium                     |   Resolution:  invalid
  Mlocked:  0                          |  
---------------------------------------+------------------------------------

Comment(by tomimo at ncircle.nullnet.fi):

 Replying to [comment:2 janne]:
 > I don't think the patch for Recorderbase is correct. The QWaitconditions
 are used as interuptible sleep. There is no need for useful mutexes in
 this usage case. Another problem with the patch is that it doesn't change
 the usage in derived classes.

 The attached patch of course doesn't fix all the places, because I noticed
 that there exists at least 15 locations around the code which has the same
 QWaitCondition-usage flaw and I wanted to test my fixes as before
 committing it all. I did however want to bring some attention to this
 problem right a way.

 I understand very well your comment about the purpose of the discussed
 code, but what I don't understand is that why would you want to keep the
 code which is obviously wrong ? This is a good example of QWaitCondition-
 usage where a wakeOne() or wakeAll() signal can be missed by mistake and
 even though the timedcondwait will eventually timeout and fix a potential
 deadlock situation, why shouldn't we fix the code to work properly in the
 first place ?

 >
 > the member variables paused and request_pause could probably be guarded
 by mutexes which should be used in the wait conditions in recorderbase.
 >
 > I'm ready to accept that our standard Qt4 port of QWaitCondition with
 adding a otherwise unused mutex might create deadlocks in some cases but
 recorderbase is a bad example.

 I'm little bit amazed to see that you have closed this ticket, even though
 you admit yourself (and in the comments of the code) that the code in
 question is propably not correct. It's not that hard to fix the detected
 errors in order to make the MythTV more stable. I'm sure you that you know
 very well that the small loop wget test proves absolutely nothing about
 the stability of the code, especially as I've been (unfortunately) able to
 get the mythbackend hang in two different hosts once or twice a day
 without fixing these QWaitCondition cases.

 I have no problem in submitting patches to MythTV, but of course I'd like
 to know that you'd be willing to accept them in some form as well.

 I'm open to suggestions how to get these fixes done and I kindly ask you
 not to close this ticket just yet.

-- 
Ticket URL: <http://svn.mythtv.org/trac/ticket/6305#comment:4>
MythTV <http://www.mythtv.org/>
MythTV


More information about the mythtv-commits mailing list