Fixed a possible crash in case replay is started and stopped in rapid sequence

This commit is contained in:
Klaus Schmidinger 2020-05-18 16:47:29 +02:00
parent dd9dd76722
commit 196785ff05
7 changed files with 92 additions and 39 deletions

11
HISTORY
View File

@ -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().

View File

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

11
menu.c
View File

@ -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:

View File

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

View File

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

View File

@ -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();

69
vdr.c
View File

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