Opened 12 years ago

Closed 12 years ago

#3794 closed patch (fixed)

NVP::EnableEdit stuck in loop waiting for GetPause

Reported by: stuartm Owned by: danielk
Priority: minor Milestone: 0.21
Component: mythtv Version: head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

When pressing an unknown combination of buttons, possibly including Pause and Edit in quick succession I managed to get the frontend stuck in the while loop in EnableEdit?. We enter a tight usleep loop waiting for NVP::GetPause?() to be true, but under some condition it seems possible that this never happens.

As a quick and simple fix, we could put a limit on the number of iterations after which EnableEdit? returns false and we print an error. However someone with more interest or time might want to look at a proper fix which prevents the underlying condition from occurring.

Attachments (1)

3794-v1.patch (3.5 KB) - added by danielk 12 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 12 years ago by Roo <roo.watt@…>

I wonder if this is related to a potential bug reported in #3636.

There does seem to be a potential for misbehaviour there relating to keypress handling.

comment:2 Changed 12 years ago by danielk

Owner: changed from Isaac Richards to danielk

Changed 12 years ago by danielk

Attachment: 3794-v1.patch added

comment:3 Changed 12 years ago by danielk

Type: defectpatch

Stuart M, can you try to break things with the attached patch applied...

comment:4 Changed 12 years ago by stuartm

I've tried to reproduce this without the patch applied and failed. Nothing unexpected happened with the patch applied though and I can't see why it wouldn't prevent the problem.

comment:5 Changed 12 years ago by danielk

Resolution: fixed
Status: newclosed

(In [14821]) Fixes #3794. Fixes a race condition which can cause 'E'dit to get stuck waiting for a pause to take effect.

The problem is that a Play could get executed while the after the 'E'dit processing function in the NVP had determined that the playback is paused. With this change GetPause?() has been renamed IsPaused?() and has an added param which tells you if someone has just executed a Play so that the pause is about to be undone, combined with locking and a check for whether we are starting editing in the Play() function this allows us to ensure that playback is really paused when we need it to be.

Tested by myself and Stuart M & reviewed by Stuart M.

Note: See TracTickets for help on using tickets.