Fixed a crash when moving a recording to a folder on a different volume

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<cRecordingsHandlerEntry> 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<c##Class *>(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 <getopt.h>
@@ -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