diff options
| -rw-r--r-- | HISTORY | 11 | ||||
| -rw-r--r-- | config.h | 10 | ||||
| -rw-r--r-- | menu.c | 11 | ||||
| -rw-r--r-- | player.c | 12 | ||||
| -rw-r--r-- | player.h | 13 | ||||
| -rw-r--r-- | skinlcars.c | 5 | ||||
| -rw-r--r-- | vdr.c | 69 |
7 files changed, 92 insertions, 39 deletions
@@ -9420,7 +9420,7 @@ Video Disk Recorder Revision History - Fixed handling the S2SatelliteDeliverySystemDescriptor for transponders broadcasting in "backwards compatibility mode" according to ETSI EN 300 468 (thanks to Onur Sentürk). -2020-05-15: +2020-05-18: Version 2.4.2 - Fixed moving channels between number groups in SVDRP's MOVC command and the Channels menu, in case a channel is moved to a higher number and into a numbered group @@ -9443,3 +9443,12 @@ Video Disk Recorder Revision History is valid (suggested by Helmut Binder). - The isSingleByte parameter in the call to getCharacterTable() is deprecated and only present for backwards compatibility. +- Fixed a possible crash in case replay is started and stopped in rapid sequence, by + adding missing locking to cControl::Control(). The caller of this function must now + provide a cMutexLock which stays alive as long as the result of this call is used. + The old version of this function is still there for backwards compatibility with + plugins, because this problem appears to occur only under very rare circumstances. + Authors of plugins that use this function should switch to the new version, because + the old one is deprecated and will be removed in a future version. + The version numbers (both VDRVERSNUM and APIVERSNUM) have been bumped to 2.4.2, so + that plugins can detect the presence of the new cControl::Control(). @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: config.h 4.16 2019/06/16 09:13:45 kls Exp $ + * $Id: config.h 4.17 2020/05/18 16:47:29 kls Exp $ */ #ifndef __CONFIG_H @@ -22,13 +22,13 @@ // VDR's own version number: -#define VDRVERSION "2.4.1" -#define VDRVERSNUM 20401 // Version * 10000 + Major * 100 + Minor +#define VDRVERSION "2.4.2" +#define VDRVERSNUM 20402 // Version * 10000 + Major * 100 + Minor // The plugin API's version number: -#define APIVERSION "2.4.1" -#define APIVERSNUM 20401 // Version * 10000 + Major * 100 + Minor +#define APIVERSION "2.4.2" +#define APIVERSNUM 20402 // Version * 10000 + Major * 100 + Minor // When loading plugins, VDR searches them by their APIVERSION, which // may be smaller than VDRVERSION in case there have been no changes to @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: menu.c 4.81 2020/04/11 09:22:05 kls Exp $ + * $Id: menu.c 4.82 2020/05/18 16:47:29 kls Exp $ */ #include "menu.h" @@ -2731,7 +2731,8 @@ eOSState cMenuRecordingEdit::DeleteMarks(void) if (buttonDeleteMarks && Interface->Confirm(tr("Delete editing marks for this recording?"))) { if (cMarks::DeleteMarksFile(recording)) { SetHelpKeys(); - if (cControl *Control = cControl::Control(true)) { + cMutexLock ControlMutexLock; + if (cControl *Control = cControl::Control(ControlMutexLock, true)) { if (const cRecording *Recording = Control->GetRecording()) { if (strcmp(recording->FileName(), Recording->FileName()) == 0) Control->ClearEditingMarks(); @@ -4457,7 +4458,11 @@ bool cMenuMain::Update(bool Force) { bool result = false; - bool NewReplaying = cControl::Control() != NULL; + bool NewReplaying = false; + { + cMutexLock ControlMutexLock; + NewReplaying = cControl::Control(ControlMutexLock) != NULL; + } if (Force || NewReplaying != replaying) { replaying = NewReplaying; // Replay control: @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: player.c 2.2 2012/04/28 11:52:50 kls Exp $ + * $Id: player.c 4.1 2020/05/18 16:47:29 kls Exp $ */ #include "player.h" @@ -70,16 +70,24 @@ cString cControl::GetHeader(void) return ""; } +#if DEPRECATED_CCONTROL cControl *cControl::Control(bool Hidden) { cMutexLock MutexLock(&mutex); return (control && (!control->hidden || Hidden)) ? control : NULL; } +#endif + +cControl *cControl::Control(cMutexLock &MutexLock, bool Hidden) +{ + MutexLock.Lock(&mutex); + return (control && (!control->hidden || Hidden)) ? control : NULL; +} void cControl::Launch(cControl *Control) { cMutexLock MutexLock(&mutex); - cControl *c = control; // keeps control from pointing to uninitialized memory + cControl *c = control; // keeps control from pointing to uninitialized memory TODO obsolete once DEPRECATED_CCONTROL is gone control = Control; delete c; } @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: player.h 4.4 2018/02/01 15:34:51 kls Exp $ + * $Id: player.h 4.5 2020/05/18 16:47:29 kls Exp $ */ #ifndef __PLAYER_H @@ -114,10 +114,21 @@ public: static void Launch(cControl *Control); static void Attach(void); static void Shutdown(void); +#define DEPRECATED_CCONTROL 1 +#if DEPRECATED_CCONTROL static cControl *Control(bool Hidden = false); + ///< Old version of this function, for backwards compatibility with plugins. + ///< Plugins should be changed to use the new version below, which does + ///< proper locking. + ///< Use of this function may result in program crashes in case replay is + ///< stopped immediately after starting it. +#endif + static cControl *Control(cMutexLock &MutexLock, bool Hidden = false); ///< Returns the current replay control (if any) in case it is currently ///< visible. If Hidden is true, the control will be returned even if it is ///< currently hidden. + ///< The given MutexLock must live as long as the replay control is accessed, + ///< and must go out of scope as soon as the control is no longer accessed. }; #endif //__PLAYER_H diff --git a/skinlcars.c b/skinlcars.c index aedca5dc..0f431631 100644 --- a/skinlcars.c +++ b/skinlcars.c @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: skinlcars.c 4.6 2017/11/08 10:10:30 kls Exp $ + * $Id: skinlcars.c 4.7 2020/05/18 16:47:29 kls Exp $ */ // "Star Trek: The Next Generation"(R) is a registered trademark of Paramount Pictures, @@ -1741,12 +1741,13 @@ void cSkinLCARSDisplayMenu::Flush(void) { if (MenuCategory() == mcMain) { cDevice *Device = cDevice::PrimaryDevice(); + cMutexLock ControlMutexLock; if (!Device->Replaying() || Device->Transferring()) { LOCK_CHANNELS_READ; const cChannel *Channel = Channels->GetByNumber(cDevice::PrimaryDevice()->CurrentChannel()); DrawLive(Channel); } - else if (cControl *Control = cControl::Control(true)) + else if (cControl *Control = cControl::Control(ControlMutexLock, true)) DrawPlay(Control); DrawTimers(); DrawDevices(); @@ -22,7 +22,7 @@ * * The project's page is at http://www.tvdr.de * - * $Id: vdr.c 4.32 2020/05/15 11:31:40 kls Exp $ + * $Id: vdr.c 4.33 2020/05/18 16:47:29 kls Exp $ */ #include <getopt.h> @@ -1196,8 +1196,19 @@ int main(int argc, char *argv[]) // Queued messages: Skins.ProcessQueuedMessages(); // User Input: - cOsdObject *Interact = Menu ? Menu : cControl::Control(); - eKeys key = Interface->GetKey(!Interact || !Interact->NeedsFastResponse()); + bool NeedsFastResponse = Menu && Menu->NeedsFastResponse(); + if (!NeedsFastResponse) { + // Must limit the scope of ControlMutexLock here to not hold the lock during the call to Interface->GetKey(). + cMutexLock ControlMutexLock; + cControl *Control = cControl::Control(ControlMutexLock); + NeedsFastResponse = Control && Control->NeedsFastResponse(); + } + eKeys key = Interface->GetKey(!NeedsFastResponse); + cOsdObject *Interact = Menu; + cMutexLock ControlMutexLock; + cControl *Control = NULL; + if (!Menu) + Interact = Control = cControl::Control(ControlMutexLock); if (ISREALKEY(key)) { EITScanner.Activity(); // Cancel shutdown countdown: @@ -1215,9 +1226,9 @@ int main(int argc, char *argv[]) bool WasMenu = Interact && Interact->IsMenu(); if (Menu) DELETE_MENU; - else if (cControl::Control()) { + else if (Control) { if (cOsd::IsOpen()) - cControl::Control()->Hide(); + Control->Hide(); else WasOpen = false; } @@ -1233,9 +1244,9 @@ int main(int argc, char *argv[]) } else if (!Menu) { IsInfoMenu = true; - if (cControl::Control()) { - cControl::Control()->Hide(); - Menu = cControl::Control()->GetInfo(); + if (Control) { + Control->Hide(); + Menu = Control->GetInfo(); if (Menu) Menu->Show(); else @@ -1252,8 +1263,8 @@ int main(int argc, char *argv[]) // Direct main menu functions: #define DirectMainFunction(function)\ { DELETE_MENU;\ - if (cControl::Control())\ - cControl::Control()->Hide();\ + if (Control)\ + Control->Hide();\ Menu = new cMenuMain(function);\ key = kNone; } // nobody else needs to see this key case kSchedule: DirectMainFunction(osSchedule); break; @@ -1267,8 +1278,8 @@ int main(int argc, char *argv[]) const char *PluginName = cRemote::GetPlugin(); if (PluginName) { DELETE_MENU; - if (cControl::Control()) - cControl::Control()->Hide(); + if (Control) + Control->Hide(); cPlugin *plugin = cPluginManager::GetPlugin(PluginName); if (plugin) { Menu = plugin->MainMenuAction(); @@ -1290,7 +1301,7 @@ int main(int argc, char *argv[]) Menu = new cDisplayChannel(NORMALKEY(key)); continue; } - else if (cDisplayChannel::IsOpen() || cControl::Control()) { + else if (cDisplayChannel::IsOpen() || Control) { Interact->ProcessKey(key); continue; } @@ -1318,8 +1329,8 @@ int main(int argc, char *argv[]) break; // Audio track control: case kAudio: - if (cControl::Control()) - cControl::Control()->Hide(); + if (Control) + Control->Hide(); if (!cDisplayTracks::IsOpen()) { DELETE_MENU; Menu = cDisplayTracks::Create(); @@ -1330,8 +1341,8 @@ int main(int argc, char *argv[]) break; // Subtitle track control: case kSubtitles: - if (cControl::Control()) - cControl::Control()->Hide(); + if (Control) + Control->Hide(); if (!cDisplaySubtitleTracks::IsOpen()) { DELETE_MENU; Menu = cDisplaySubtitleTracks::Create(); @@ -1343,7 +1354,7 @@ int main(int argc, char *argv[]) // Pausing live video: case kPlayPause: case kPause: - if (!cControl::Control()) { + if (!Control) { DELETE_MENU; if (Setup.PauseKeyHandling) { if (Setup.PauseKeyHandling > 1 || Interface->Confirm(tr("Pause live video?"))) { @@ -1356,7 +1367,7 @@ int main(int argc, char *argv[]) break; // Instant recording: case kRecord: - if (!cControl::Control()) { + if (!Control) { if (Setup.RecordKeyHandling) { if (Setup.RecordKeyHandling > 1 || Interface->Confirm(tr("Start recording?"))) { if (cRecordControls::Start()) @@ -1395,15 +1406,16 @@ int main(int argc, char *argv[]) break; default: break; } - Interact = Menu ? Menu : cControl::Control(); // might have been closed in the mean time + Interact = Menu ? Menu : Control; // might have been closed in the mean time if (Interact) { LastInteract = Now; eOSState state = Interact->ProcessKey(key); - if (state == osUnknown && Interact != cControl::Control()) { - if (ISMODELESSKEY(key) && cControl::Control()) { - state = cControl::Control()->ProcessKey(key); + if (state == osUnknown && Interact != Control) { + if (ISMODELESSKEY(key) && Control) { + state = Control->ProcessKey(key); if (state == osEnd) { // let's not close a menu when replay ends: + Control = NULL; cControl::Shutdown(); continue; } @@ -1422,15 +1434,18 @@ int main(int argc, char *argv[]) break; case osRecordings: DELETE_MENU; + Control = NULL; cControl::Shutdown(); Menu = new cMenuMain(osRecordings, true); break; case osReplay: DELETE_MENU; + Control = NULL; cControl::Shutdown(); cControl::Launch(new cReplayControl); break; case osStopReplay: DELETE_MENU; + Control = NULL; cControl::Shutdown(); break; case osPlugin: DELETE_MENU; @@ -1441,8 +1456,10 @@ int main(int argc, char *argv[]) case osBack: case osEnd: if (Interact == Menu) DELETE_MENU; - else + else { + Control = NULL; cControl::Shutdown(); + } break; default: ; } @@ -1487,6 +1504,7 @@ int main(int argc, char *argv[]) // Instant resume of the last viewed recording: case kPlay: if (cReplayControl::LastReplayed()) { + Control = NULL; cControl::Shutdown(); cControl::Launch(new cReplayControl); } @@ -1512,6 +1530,7 @@ int main(int argc, char *argv[]) int NewPrimaryDVB = Setup.PrimaryDVB; if (NewPrimaryDVB != OldPrimaryDVB) { DELETE_MENU; + Control = NULL; cControl::Shutdown(); Skins.QueueMessage(mtInfo, tr("Switching primary DVB...")); cOsdProvider::Shutdown(); @@ -1532,7 +1551,7 @@ int main(int argc, char *argv[]) ShutdownHandler.countdown.Cancel(); } - if (!cControl::Control() && !cRecordControls::Active() && !RecordingsHandler.Active() && (Now - cRemote::LastActivity()) > ACTIVITYTIMEOUT) { + if (!Control && !cRecordControls::Active() && !RecordingsHandler.Active() && (Now - cRemote::LastActivity()) > ACTIVITYTIMEOUT) { // Shutdown: // Check whether VDR will be ready for shutdown in SHUTDOWNWAIT seconds: time_t Soon = Now + SHUTDOWNWAIT; |