[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