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; | 
