Opened 9 years ago

Closed 9 years ago

#8863 closed defect (fixed)

'myth_system' doesn't disable lirc input when running subprocesses

Reported by: Ian Clark <mrrooster@…> Owned by: beirdo
Priority: minor Milestone: 0.24
Component: MythTV - General Version: Master Head
Severity: low Keywords: mythfrontend mythwelcome
Cc: Ticket locked: no

Description

When running a subprocess via EXEC, eg XBMC, Myth doesn't suspend lirc input, with the result that upon exiting the sub process Myth receives all the pending inputs in rapid sucession. This results in random items being selects, rapid channel changing, etc.

This can also be seen with mythwelcome.

Steps to reproduce:

  1. Configure mythfrontend to allow exit via the menu
  2. Start mythwelcome
  3. Start mythfrontend from mythwelcome
  4. Press the back button to bring up the mythfrontend menu. (This is the button on my remote anyway.)
  5. Select 'Yes, Exit now'
  6. Mythfrontend will restart due to receiving the remote input from step 5.

The attach patch modifies mythsystem.cpp to correctly lock lircd input.

SVN Revision: 26124

Attachments (4)

mythsystem_liirc_locking.diff (1.1 KB) - added by Ian Clark <mrrooster@…> 9 years ago.
svn diff file
8863_v2.patch (1.3 KB) - added by beirdo 9 years ago.
8863_v2.1.patch (950 bytes) - added by Ian Clark <mrrooster@…> 9 years ago.
0001-Patch-v3-8863-fix-LIRC-JS-Menu-locks-in-myth_system.patch (7.8 KB) - added by beirdo 9 years ago.

Download all attachments as: .zip

Change History (18)

Changed 9 years ago by Ian Clark <mrrooster@…>

svn diff file

comment:1 Changed 9 years ago by beirdo

Owner: set to beirdo
Status: newassigned

Changed 9 years ago by beirdo

Attachment: 8863_v2.patch added

comment:2 Changed 9 years ago by beirdo

Status: assignedinfoneeded

Please try this patch and let me know if it does what you need.

It seems that the configuration wasn't being passed in quite properly.

comment:3 Changed 9 years ago by Ian Clark <mrrooster@…>

Status: infoneededassigned

Hi Beirdo,

Cheers for the patch.

It still doesn't work unfortunatly. The config does seem to be passed through now, however you also need to move the LircEventLock? object out of 'myth_system_pre_flags' into 'myth_system' otherwise it goes out of scope (and hence unlocks lircd) before the subprocess is forked.

I've attached a patch against your changes to illustrate what I mean.

Cheers,

Ian

Changed 9 years ago by Ian Clark <mrrooster@…>

Attachment: 8863_v2.1.patch added

comment:4 Changed 9 years ago by beirdo

Please use unified diffs (diff -u).

Anyways, I have modified the code (again), and have attached a new patch here. The scoping is problematic, but needs to be dealt with in this way as we have at least one consumer calling with abort support, and this requires the locks to be accessible outside of myth_system in that particular case.

Please try this and let me know.

Changed 9 years ago by beirdo

comment:5 Changed 9 years ago by beirdo

Status: assignedinfoneeded

comment:6 Changed 9 years ago by beirdo

Milestone: unknown0.24

comment:7 Changed 9 years ago by beirdo

Type: patchdefect

comment:8 Changed 9 years ago by Ian Clark <mrrooster@…>

Status: infoneededassigned

Yup, that works great. Cheers. :)

comment:9 Changed 9 years ago by beirdo

Resolution: fixed
Status: assignedclosed

(In [26141]) Changes the locks for LIRC and Joystick Menu in myth_system to have an appropriate scope so the locking works. Also using CONFIG_LIRC and CONFIG_JOYSTICK_MENU instead of USING_ which were not being setup in the .pro file. The CONFIG_ are setup in mythconfig.h.

Fixes #8863

comment:10 Changed 9 years ago by sphery

Resolution: fixed
Status: closednew

After further research by beirdo, cpinkham, and myself, we've figure out that this was actually caused by the move of myth_system() from libmythui to libmythdb in [24623]. Technically, that change created a circular dependency between libmythui and libmythdb, but because USE_LIRC and USE_JOYSTICK_MENU weren't defined in libmythdb.pro, the code to lock LIRC and joystick events was never compiled (meaning libmythdb never actually used libmythui code, and myth_system didn't disable LIRC or joystick events).

[26141] made the circular dependency visible, so this needs to be changed to use the same approach Chris used to disable UI updates from myth_system()--by sending an event. This will allow removing all the #if/#ifdef lines for LIRC and joystick events from myth_system() and may also allow removal/simplification of some of the LIRC and joystick-related locking code in libmythui.

And, in the interim compilation with --enable-symbol-visibility is broken, so until this is fixed, users should compile without --enable-symbol-visibility.

comment:11 Changed 9 years ago by beirdo

Status: newaccepted

comment:12 in reply to:  10 Changed 9 years ago by paulh

Replying to mdean:

[26141] made the circular dependency visible, so this needs to be changed to use the same approach Chris used to disable UI updates from myth_system()--by sending an event. This will allow removing all the #if/#ifdef lines for LIRC and joystick events from myth_system() and may also allow removal/simplification of some of the LIRC and joystick-related locking code in libmythui.

Unless something has changed the problem with sending an event is you need to make sure the event has been received and processed before starting the external process because myth_system() is supposed to block the main thread while the external process is running unless the MYTH_SYSTEM_DONT_BLOCK_PARENT flag is used. You can use dispatchNow() but that is depreciated.

comment:13 Changed 9 years ago by beirdo

See reply on the mailing list.

comment:14 Changed 9 years ago by beirdo

Resolution: fixed
Status: acceptedclosed

(In [26166]) This rather large commit removes the unintentional circular dependency we managed to make when moving myth_system to libmythdb in [24623].

Replace the calls to the lirc and joystick menu locks with events to the main window. This collapses the flags to myth_system some, which touches a lot of consumers to use the new flags. Changed the flags to an enum (at the suggestion of cpinkham).

Also removes the classes and events used to do those locks as they are no longer used anywhere.

Finally, make the blocked threads timeout so they can processEvents every 100ms, then reblock. This allows the LIRC presses, etc during that time to get ignored properly and in a relatively timely manner.

Fixes #8863

Note: See TracTickets for help on using tickets.