From 08066065e316682da10249b4edcc19b7d5226e59 Mon Sep 17 00:00:00 2001 From: Klaus Schmidinger Date: Tue, 13 Dec 2016 13:54:00 +0100 Subject: [PATCH] Fixed a crash when moving a recording to a folder on a different volume --- HISTORY | 6 +++++- menu.c | 12 ++++++------ recording.c | 49 +++++++++++++++++++++++++++---------------------- recording.h | 14 +++++--------- tools.h | 4 ++-- vdr.c | 5 +---- 6 files changed, 46 insertions(+), 44 deletions(-) diff --git a/HISTORY b/HISTORY index 18c29885..290d5b44 100644 --- a/HISTORY +++ b/HISTORY @@ -8828,7 +8828,7 @@ Video Disk Recorder Revision History - Empty adaptation field TS packets are now skipped when recording (thanks to Christopher Reimer, based on the "AFFcleaner" by Stefan Pöschel). -2016-12-11: Version 2.3.2 +2016-12-13: Version 2.3.2 - Fixed a crash when deleting a recording (reported by Oliver Endriss). - Fixed an overflow of PIDs in a receiver (thanks to Robert Hannebauer). @@ -8847,3 +8847,7 @@ Video Disk Recorder Revision History - Fixed setting the current item and counter values in the Recordings menu after deleting the last recording in a subfolder. - Fixed a crash when deleting a recording that is currently being replayed. +- Fixed a crash when moving a recording to a folder on a different volume. + The cRecordingsHandler now performs its actual operations in a separate thread, + thus avoiding locking problems and reducing the time between subsequent + operations. diff --git a/menu.c b/menu.c index 598fc3d3..48889a48 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.17 2016/12/11 12:43:55 kls Exp $ + * $Id: menu.c 4.18 2016/12/13 12:49:10 kls Exp $ */ #include "menu.h" @@ -2687,14 +2687,15 @@ eOSState cMenuRecordingEdit::ApplyChanges(void) cRecordings *Recordings = cRecordings::GetRecordingsWrite(StateKey); cRecording *Recording = Recordings->GetByName(recording->FileName()); if (!Recording) { + StateKey.Remove(false); Skins.Message(mtWarning, tr("Recording vanished!")); return osBack; } bool Modified = false; if (priority != recording->Priority() || lifetime != recording->Lifetime()) { if (!Recording->ChangePriorityLifetime(priority, lifetime)) { - Skins.Message(mtError, tr("Error while changing priority/lifetime!")); StateKey.Remove(Modified); + Skins.Message(mtError, tr("Error while changing priority/lifetime!")); return osContinue; } Modified = true; @@ -2707,8 +2708,8 @@ eOSState cMenuRecordingEdit::ApplyChanges(void) NewName.CompactChars(FOLDERDELIMCHAR); if (strcmp(NewName, Recording->Name())) { if (!Recording->ChangeName(NewName)) { - Skins.Message(mtError, tr("Error while changing folder/name!")); StateKey.Remove(Modified); + Skins.Message(mtError, tr("Error while changing folder/name!")); return osContinue; } Modified = true; @@ -3095,13 +3096,12 @@ eOSState cMenuRecordings::Delete(void) if (const cRecording *Recording = Recordings->GetByName(ri->Recording()->FileName())) { FileName = Recording->FileName(); if (RecordingsHandler.GetUsage(FileName)) { - if (Interface->Confirm(tr("Recording is being edited - really delete?"))) - RecordingsHandler.Del(FileName); - else + if (!Interface->Confirm(tr("Recording is being edited - really delete?"))) return osContinue; } } } + RecordingsHandler.Del(FileName); // must do this w/o holding a lock, because the cleanup section in cDirCopier::Action() might request one! if (cReplayControl::NowReplaying() && strcmp(cReplayControl::NowReplaying(), FileName) == 0) cControl::Shutdown(); cRecordings *Recordings = cRecordings::GetRecordingsWrite(recordingsStateKey); diff --git a/recording.c b/recording.c index a847c7d1..9ec3ea9d 100644 --- a/recording.c +++ b/recording.c @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: recording.c 4.4 2015/09/09 10:21:58 kls Exp $ + * $Id: recording.c 4.5 2016/12/13 13:39:09 kls Exp $ */ #include "recording.h" @@ -1701,7 +1701,7 @@ void cDirCopier::Action(void) int To = -1; size_t BufferSize = BUFSIZ; while (Running()) { - // Suspend cutting if we have severe throughput problems: + // Suspend copying if we have severe throughput problems: if (Throttled()) { cCondWait::SleepMs(100); continue; @@ -1900,6 +1900,7 @@ bool cRecordingsHandlerEntry::Active(bool &Error) cRecordingsHandler RecordingsHandler; cRecordingsHandler::cRecordingsHandler(void) +:cThread("recordings handler") { finished = true; error = false; @@ -1907,6 +1908,23 @@ cRecordingsHandler::cRecordingsHandler(void) cRecordingsHandler::~cRecordingsHandler() { + Cancel(3); +} + +void cRecordingsHandler::Action(void) +{ + while (Running()) { + { + cMutexLock MutexLock(&mutex); + while (cRecordingsHandlerEntry *r = operations.First()) { + if (!r->Active(error)) + operations.Del(r); + } + if (!operations.Count()) + break; + } + cCondWait::SleepMs(100); + } } cRecordingsHandlerEntry *cRecordingsHandler::Get(const char *FileName) @@ -1934,8 +1952,7 @@ bool cRecordingsHandler::Add(int Usage, const char *FileNameSrc, const char *Fil Usage |= ruPending; operations.Add(new cRecordingsHandlerEntry(Usage, FileNameSrc, FileNameDst)); finished = false; - Active(); // start it right away if possible - LOCK_RECORDINGS_WRITE; // to trigger a state change + Start(); return true; } else @@ -1955,17 +1972,17 @@ bool cRecordingsHandler::Add(int Usage, const char *FileNameSrc, const char *Fil void cRecordingsHandler::Del(const char *FileName) { cMutexLock MutexLock(&mutex); - if (cRecordingsHandlerEntry *r = Get(FileName)) { + if (cRecordingsHandlerEntry *r = Get(FileName)) operations.Del(r); - LOCK_RECORDINGS_WRITE; // to trigger a state change - } } void cRecordingsHandler::DelAll(void) { - cMutexLock MutexLock(&mutex); - operations.Clear(); - LOCK_RECORDINGS_WRITE; // to trigger a state change + { + cMutexLock MutexLock(&mutex); + operations.Clear(); + } + Cancel(3); } int cRecordingsHandler::GetUsage(const char *FileName) @@ -1976,18 +1993,6 @@ int cRecordingsHandler::GetUsage(const char *FileName) return ruNone; } -bool cRecordingsHandler::Active(void) -{ - cMutexLock MutexLock(&mutex); - while (cRecordingsHandlerEntry *r = operations.First()) { - if (r->Active(error)) - return true; - else - operations.Del(r); - } - return false; -} - bool cRecordingsHandler::Finished(bool &Error) { cMutexLock MutexLock(&mutex); diff --git a/recording.h b/recording.h index cebd2ee0..b649f6fd 100644 --- a/recording.h +++ b/recording.h @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: recording.h 4.3 2015/08/29 14:12:14 kls Exp $ + * $Id: recording.h 4.4 2016/12/13 13:12:12 kls Exp $ */ #ifndef __RECORDING_H @@ -302,16 +302,18 @@ DEF_LIST_LOCK2(Recordings, DeletedRecordings); class cRecordingsHandlerEntry; -class cRecordingsHandler { +class cRecordingsHandler : public cThread { private: cMutex mutex; cList operations; bool finished; bool error; cRecordingsHandlerEntry *Get(const char *FileName); +protected: + virtual void Action(void); public: cRecordingsHandler(void); - ~cRecordingsHandler(); + virtual ~cRecordingsHandler(); bool Add(int Usage, const char *FileNameSrc, const char *FileNameDst = NULL); ///< Adds the given FileNameSrc to the recordings handler for (later) ///< processing. Usage can be either ruCut, ruMove or ruCopy. FileNameDst @@ -329,12 +331,6 @@ public: ///< Deletes/terminates all operations. int GetUsage(const char *FileName); ///< Returns the usage type for the given FileName. - bool Active(void); - ///< Checks whether there is currently any operation running and starts - ///> the next one form the list if the previous one has finished. - ///< This function must be called regularly to trigger switching to the - ///< next operation in the list. - ///< Returns true if there are any operations in the list. bool Finished(bool &Error); ///< Returns true if all operations in the list have been finished. ///< If there have been any errors, Errors will be set to true. diff --git a/tools.h b/tools.h index 1563db92..f2ba3157 100644 --- a/tools.h +++ b/tools.h @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: tools.h 4.3 2015/09/06 10:45:54 kls Exp $ + * $Id: tools.h 4.4 2016/12/13 12:13:46 kls Exp $ */ #ifndef __TOOLS_H @@ -609,7 +609,7 @@ public: \ else \ list = c##Class::Get##Name##Read(stateKey); \ } \ - ~c##Name##Lock() { stateKey.Remove(); } \ + ~c##Name##Lock() { if (list) stateKey.Remove(); } \ const c##Class *Name(void) const { return list; } \ c##Class *Name(void) { return const_cast(list); } \ } diff --git a/vdr.c b/vdr.c index 6b0bf2b0..16a6c326 100644 --- a/vdr.c +++ b/vdr.c @@ -22,7 +22,7 @@ * * The project's page is at http://www.tvdr.de * - * $Id: vdr.c 4.7 2015/09/11 08:02:50 kls Exp $ + * $Id: vdr.c 4.8 2016/12/13 13:13:10 kls Exp $ */ #include @@ -1480,9 +1480,6 @@ int main(int argc, char *argv[]) ShutdownHandler.countdown.Cancel(); } - // Keep the recordings handler alive: - RecordingsHandler.Active(); - if ((Now - LastInteract) > ACTIVITYTIMEOUT && !cRecordControls::Active() && !RecordingsHandler.Active() && (Now - cRemote::LastActivity()) > ACTIVITYTIMEOUT) { // Handle housekeeping tasks