diff --git a/HISTORY b/HISTORY index c8cf54f3..2f4feb33 100644 --- a/HISTORY +++ b/HISTORY @@ -9135,3 +9135,6 @@ Video Disk Recorder Revision History - Updated links in the INSTALL file (thanks to Chris Mayo). - Fixed detecting whether a CAM replies to queries, which didn't work on some systems since the implementation of RI_HOST_CONTROL (reported by Daniel Scheller). +- Added some missing locks when calling functions from cStatus or cSkin*, and added + some text to status.h and skins.h, explaining the locking situation when such + functions are called. diff --git a/menu.c b/menu.c index 8f83ae00..209482a6 100644 --- a/menu.c +++ b/menu.c @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: menu.c 4.38 2017/06/20 15:02:39 kls Exp $ + * $Id: menu.c 4.39 2017/06/21 09:19:59 kls Exp $ */ #include "menu.h" @@ -1714,7 +1714,10 @@ eOSState cMenuWhatsOn::ProcessKey(eKeys Key) for (cOsdItem *item = First(); item; item = Next(item)) { if (((cMenuScheduleItem *)item)->channel->Number() == cDevice::CurrentChannel()) { SetCurrent(item); - Display(); + { + LOCK_SCHEDULES_READ; + Display(); + } LOCK_CHANNELS_READ; SetHelpKeys(Channels); break; @@ -1733,8 +1736,10 @@ eOSState cMenuWhatsOn::ProcessKey(eKeys Key) } } else if (!HasSubMenu()) { - if (HadSubMenu && Update()) + if (HadSubMenu && Update()) { + LOCK_SCHEDULES_READ; Display(); + } if (Key != kNone) { LOCK_CHANNELS_READ; SetHelpKeys(Channels); @@ -1953,32 +1958,30 @@ eOSState cMenuSchedule::Number(void) eOSState cMenuSchedule::Record(void) { if (cMenuScheduleItem *item = (cMenuScheduleItem *)Get(Current())) { - { - LOCK_TIMERS_WRITE; - LOCK_CHANNELS_READ; - LOCK_SCHEDULES_READ; - Timers->SetExplicitModify(); - if (item->timerMatch == tmFull) { - if (cTimer *Timer = Timers->GetMatch(item->event)) - return AddSubMenu(new cMenuEditTimer(Timer)); - } - cTimer *Timer = new cTimer(item->event); - if (Setup.SVDRPPeering && *Setup.SVDRPDefaultHost) - Timer->SetRemote(Setup.SVDRPDefaultHost); - if (cTimer *t = Timers->GetTimer(Timer)) { - delete Timer; - Timer = t; - return AddSubMenu(new cMenuEditTimer(Timer)); - } - if (Timer->Matches(0, false, NEWTIMERLIMIT)) - return AddSubMenu(new cMenuEditTimer(Timer, true)); - Timers->Add(Timer); - Timers->SetModified(); - if (!HandleRemoteModifications(Timer)) { - // must add the timer before HandleRemoteModifications to get proper log messages with timer ids - Timers->Del(Timer); - } - } + LOCK_TIMERS_WRITE; + LOCK_CHANNELS_READ; + LOCK_SCHEDULES_READ; + Timers->SetExplicitModify(); + if (item->timerMatch == tmFull) { + if (cTimer *Timer = Timers->GetMatch(item->event)) + return AddSubMenu(new cMenuEditTimer(Timer)); + } + cTimer *Timer = new cTimer(item->event); + if (Setup.SVDRPPeering && *Setup.SVDRPDefaultHost) + Timer->SetRemote(Setup.SVDRPDefaultHost); + if (cTimer *t = Timers->GetTimer(Timer)) { + delete Timer; + Timer = t; + return AddSubMenu(new cMenuEditTimer(Timer)); + } + if (Timer->Matches(0, false, NEWTIMERLIMIT)) + return AddSubMenu(new cMenuEditTimer(Timer, true)); + Timers->Add(Timer); + Timers->SetModified(); + if (!HandleRemoteModifications(Timer)) { + // must add the timer before HandleRemoteModifications to get proper log messages with timer ids + Timers->Del(Timer); + } if (HasSubMenu()) CloseSubMenu(); if (Update()) @@ -2077,8 +2080,10 @@ eOSState cMenuSchedule::ProcessKey(eKeys Key) Set(Timers, Channels, Channel, true); } } - else if (HadSubMenu && Update()) + else if (HadSubMenu && Update()) { + LOCK_SCHEDULES_READ; Display(); + } if (Key != kNone) SetHelpKeys(); } @@ -3189,8 +3194,10 @@ eOSState cMenuRecordings::ProcessKey(eKeys Key) return state; // closes all recording menus except for the top one Set(); // this is the top level menu, so we refresh it... Open(true); // ...and open any necessary submenus to show the new name - if (!HasSubMenu()) + if (!HasSubMenu()) { + LOCK_RECORDINGS_READ; Display(); + } path = NULL; fileName = NULL; } diff --git a/skins.h b/skins.h index 2286087b..f994d47d 100644 --- a/skins.h +++ b/skins.h @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: skins.h 4.1 2015/09/10 11:19:48 kls Exp $ + * $Id: skins.h 4.2 2017/06/21 09:40:39 kls Exp $ */ #ifndef __SKINS_H @@ -21,6 +21,17 @@ #include "timers.h" #include "tools.h" +// Several member functions of the following classes are called with a pointer to +// an object from a global list (cTimer, cChannel, cRecording or cEvent). In these +// cases the core VDR code holds a lock on the respective list. The called function +// may itself set a read lock (not a write lock!) on this list, because read locks +// can be nested. It may also set read locks (not write locks!) on higher order lists. +// For instance, a function that is called with a cChannel may lock cRecordings and/or +// cSchedules (which contains cEvent objects), but not cTimers. If a plugin needs to +// set locks of its own (on mutexes defined inside the plugin code), it shall do so +// after setting any locks on VDR's global lists, and it shall always set these +// locks in the same sequence, to avoid deadlocks. + enum eMessageType { mtStatus = 0, mtInfo, mtWarning, mtError }; // will be used to calculate color offsets! class cSkinDisplay { diff --git a/status.h b/status.h index 6d1b9df5..32a9c2ad 100644 --- a/status.h +++ b/status.h @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: status.h 4.1 2015/08/02 10:34:44 kls Exp $ + * $Id: status.h 4.2 2017/06/21 09:40:20 kls Exp $ */ #ifndef __STATUS_H @@ -15,6 +15,17 @@ #include "player.h" #include "tools.h" +// Several member functions of the following classes are called with a pointer to +// an object from a global list (cTimer, cChannel, cRecording or cEvent). In these +// cases the core VDR code holds a lock on the respective list. The called function +// may itself set a read lock (not a write lock!) on this list, because read locks +// can be nested. It may also set read locks (not write locks!) on higher order lists. +// For instance, a function that is called with a cChannel may lock cRecordings and/or +// cSchedules (which contains cEvent objects), but not cTimers. If a plugin needs to +// set locks of its own (on mutexes defined inside the plugin code), it shall do so +// after setting any locks on VDR's global lists, and it shall always set these +// locks in the same sequence, to avoid deadlocks. + enum eTimerChange { tcMod, tcAdd, tcDel }; // tcMod is obsolete and no longer used! class cTimer;