Opened 13 years ago

Closed 13 years ago

#1826 closed patch (fixed)

consecutive recordings in mythtv-eit broken

Reported by: Janne <janne-mythtv@…> Owned by: danielk
Priority: minor Milestone: 0.20
Component: dvb Version: head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

Daniel,

the problem is probably only a timing issue. Adding one debug cerr after StopRecording? was working too, but WaitingForEventThreadSleep?() is probably the correct solution?

see attached patch

Attachments (1)

consecutive_recordings.diff (422 bytes) - added by Janne <janne-mythtv@…> 13 years ago.

Download all attachments as: .zip

Change History (4)

Changed 13 years ago by Janne <janne-mythtv@…>

Attachment: consecutive_recordings.diff added

comment:1 Changed 13 years ago by danielk

Milestone: 0.20
Version: head

So you think a tuning event is getting inserted while the StopRecording?() method call is returning? This seems unlikely.

comment:2 Changed 13 years ago by Janne <janne-mythtv@…>

I don't know. But it seems indeed unlikely.

I describe the error in more detail.

TVRec::FinishedRecording? is called twice. Once in TVRec::TuningShutdowns?() (line 3259) and once in TeardownRecorder? (line 977). TeardownRecorder? is also called from TuningShutdowns?. This is identical in trunk and mythtv-eit.

In TeardownRecorder? FinishedRecording? is called with curRecording as argument. In mythtv-eit is curRecording at this time already the next recording. In trunk it is still the old one.

Adding just a little delay before setting curRecording in StartRecording? fixes this. A debug cerr is enough, I haven't seen the WaitingForEventThreadSleep?() in StopRecording?, so I thought it was missing.

It seems that the thread synchronization is not correct. I can investigate further if you want.

comment:3 Changed 13 years ago by danielk

Resolution: fixed
Status: newclosed

(In [9939]) Fixes #1826, breakage of consecutive recording in mythtv-eit branch.

This turns out to be a fairly simple race condition.

In the EIT branch we were calling tuningRequest.deque() before TuningShutdowns?(). But this is unsafe because we release the stateChangeLock when shutting down a recorder; so if another thread is in WaitForEventThreadSleep?(), it can exit the wait before we finish tearing down the recorder. The fix is to just use front() to get the tuning data early enough for our uses in HandleTuning?() and call deque() after we've returned from TuningShutdowns?().

The patch itself is two lines of code change plus two lines of exposition.

Note: See TracTickets for help on using tickets.