Added some missing locks when calling functions from cStatus or cSkin*

This commit is contained in:
Klaus Schmidinger 2017-06-21 09:40:39 +02:00
parent 699c073d85
commit 1bce499383
4 changed files with 65 additions and 33 deletions

View File

@ -9135,3 +9135,6 @@ Video Disk Recorder Revision History
- Updated links in the INSTALL file (thanks to Chris Mayo). - 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 - 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). 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.

69
menu.c
View File

@ -4,7 +4,7 @@
* See the main source file 'vdr.c' for copyright information and * See the main source file 'vdr.c' for copyright information and
* how to reach the author. * 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" #include "menu.h"
@ -1714,7 +1714,10 @@ eOSState cMenuWhatsOn::ProcessKey(eKeys Key)
for (cOsdItem *item = First(); item; item = Next(item)) { for (cOsdItem *item = First(); item; item = Next(item)) {
if (((cMenuScheduleItem *)item)->channel->Number() == cDevice::CurrentChannel()) { if (((cMenuScheduleItem *)item)->channel->Number() == cDevice::CurrentChannel()) {
SetCurrent(item); SetCurrent(item);
Display(); {
LOCK_SCHEDULES_READ;
Display();
}
LOCK_CHANNELS_READ; LOCK_CHANNELS_READ;
SetHelpKeys(Channels); SetHelpKeys(Channels);
break; break;
@ -1733,8 +1736,10 @@ eOSState cMenuWhatsOn::ProcessKey(eKeys Key)
} }
} }
else if (!HasSubMenu()) { else if (!HasSubMenu()) {
if (HadSubMenu && Update()) if (HadSubMenu && Update()) {
LOCK_SCHEDULES_READ;
Display(); Display();
}
if (Key != kNone) { if (Key != kNone) {
LOCK_CHANNELS_READ; LOCK_CHANNELS_READ;
SetHelpKeys(Channels); SetHelpKeys(Channels);
@ -1953,32 +1958,30 @@ eOSState cMenuSchedule::Number(void)
eOSState cMenuSchedule::Record(void) eOSState cMenuSchedule::Record(void)
{ {
if (cMenuScheduleItem *item = (cMenuScheduleItem *)Get(Current())) { if (cMenuScheduleItem *item = (cMenuScheduleItem *)Get(Current())) {
{ LOCK_TIMERS_WRITE;
LOCK_TIMERS_WRITE; LOCK_CHANNELS_READ;
LOCK_CHANNELS_READ; LOCK_SCHEDULES_READ;
LOCK_SCHEDULES_READ; Timers->SetExplicitModify();
Timers->SetExplicitModify(); if (item->timerMatch == tmFull) {
if (item->timerMatch == tmFull) { if (cTimer *Timer = Timers->GetMatch(item->event))
if (cTimer *Timer = Timers->GetMatch(item->event)) return AddSubMenu(new cMenuEditTimer(Timer));
return AddSubMenu(new cMenuEditTimer(Timer)); }
} cTimer *Timer = new cTimer(item->event);
cTimer *Timer = new cTimer(item->event); if (Setup.SVDRPPeering && *Setup.SVDRPDefaultHost)
if (Setup.SVDRPPeering && *Setup.SVDRPDefaultHost) Timer->SetRemote(Setup.SVDRPDefaultHost);
Timer->SetRemote(Setup.SVDRPDefaultHost); if (cTimer *t = Timers->GetTimer(Timer)) {
if (cTimer *t = Timers->GetTimer(Timer)) { delete Timer;
delete Timer; Timer = t;
Timer = t; return AddSubMenu(new cMenuEditTimer(Timer));
return AddSubMenu(new cMenuEditTimer(Timer)); }
} if (Timer->Matches(0, false, NEWTIMERLIMIT))
if (Timer->Matches(0, false, NEWTIMERLIMIT)) return AddSubMenu(new cMenuEditTimer(Timer, true));
return AddSubMenu(new cMenuEditTimer(Timer, true)); Timers->Add(Timer);
Timers->Add(Timer); Timers->SetModified();
Timers->SetModified(); if (!HandleRemoteModifications(Timer)) {
if (!HandleRemoteModifications(Timer)) { // must add the timer before HandleRemoteModifications to get proper log messages with timer ids
// must add the timer before HandleRemoteModifications to get proper log messages with timer ids Timers->Del(Timer);
Timers->Del(Timer); }
}
}
if (HasSubMenu()) if (HasSubMenu())
CloseSubMenu(); CloseSubMenu();
if (Update()) if (Update())
@ -2077,8 +2080,10 @@ eOSState cMenuSchedule::ProcessKey(eKeys Key)
Set(Timers, Channels, Channel, true); Set(Timers, Channels, Channel, true);
} }
} }
else if (HadSubMenu && Update()) else if (HadSubMenu && Update()) {
LOCK_SCHEDULES_READ;
Display(); Display();
}
if (Key != kNone) if (Key != kNone)
SetHelpKeys(); SetHelpKeys();
} }
@ -3189,8 +3194,10 @@ eOSState cMenuRecordings::ProcessKey(eKeys Key)
return state; // closes all recording menus except for the top one return state; // closes all recording menus except for the top one
Set(); // this is the top level menu, so we refresh it... Set(); // this is the top level menu, so we refresh it...
Open(true); // ...and open any necessary submenus to show the new name Open(true); // ...and open any necessary submenus to show the new name
if (!HasSubMenu()) if (!HasSubMenu()) {
LOCK_RECORDINGS_READ;
Display(); Display();
}
path = NULL; path = NULL;
fileName = NULL; fileName = NULL;
} }

13
skins.h
View File

@ -4,7 +4,7 @@
* See the main source file 'vdr.c' for copyright information and * See the main source file 'vdr.c' for copyright information and
* how to reach the author. * 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 #ifndef __SKINS_H
@ -21,6 +21,17 @@
#include "timers.h" #include "timers.h"
#include "tools.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! enum eMessageType { mtStatus = 0, mtInfo, mtWarning, mtError }; // will be used to calculate color offsets!
class cSkinDisplay { class cSkinDisplay {

View File

@ -4,7 +4,7 @@
* See the main source file 'vdr.c' for copyright information and * See the main source file 'vdr.c' for copyright information and
* how to reach the author. * 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 #ifndef __STATUS_H
@ -15,6 +15,17 @@
#include "player.h" #include "player.h"
#include "tools.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! enum eTimerChange { tcMod, tcAdd, tcDel }; // tcMod is obsolete and no longer used!
class cTimer; class cTimer;