Opened 11 years ago

Closed 11 years ago

#5481 closed patch (fixed)

PATCH: Occasional deadlock in Mainserver::ProcessRequestThread [SVN: Trunk]

Reported by: tomimo@… Owned by: danielk
Priority: minor Milestone: 0.22
Component: mythtv Version: head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

It seems that the mainserver.cpp uses QWaitCondition-variables without proper predicative/counter. This means that if a pthread_cond_signal() is sent before the other thread has already called QT's waitCond.wait(), the receiver will miss the signal alltogether (see http://doc.trolltech.com/4.3/qwaitcondition.html examples of using counter with waitCond.wait()).

The attached patch fixes this deadlock problem for good.

Attachments (2)

conditional.patch (3.6 KB) - added by tomimo@… 11 years ago.
Fixes an occasional deadlock in Mainserver::ProcessRequestThread?
5481-v1.patch (4.0 KB) - added by danielk 11 years ago.
Updated to trunk, completely untested/unreviewed.

Download all attachments as: .zip

Change History (5)

Changed 11 years ago by tomimo@…

Attachment: conditional.patch added

Fixes an occasional deadlock in Mainserver::ProcessRequestThread?

comment:1 Changed 11 years ago by danielk

Owner: changed from Isaac Richards to danielk
Status: newassigned

I don't know if I will apply this patch as is, but this is definitely a bug.

comment:2 Changed 11 years ago by danielk

Milestone: unknown0.22
Status: assignedaccepted

Changed 11 years ago by danielk

Attachment: 5481-v1.patch added

Updated to trunk, completely untested/unreviewed.

comment:3 Changed 11 years ago by danielk

Resolution: fixed
Status: acceptedclosed

(In [19511]) Fixes #5481. There is in fact no deadlock, whoever wrote this code was very careful to only issue wakes and waits so that a predicative/counter was not needed. However, there was a missing threadPoolCond.wakeXXX() call causing an uneccesary 10 ms delay in handling some requests, and just to avoid a small change mucking up the careful orchestration I've changed the wakeOne() calls to wakeAll() calls which are a little more robust (however they are NOT needed for the current code -- it's just to protect against possible changes in Qt customEvent() handling causing problems in the future.)

Note: See TracTickets for help on using tickets.