[Core] Archive corrupt torrent.state on load

If the torrent.state was corrupted then loading would create a new
state with no backup to examine.

The solution is to use the archive function to save a copy of the
torrent.state.

Added a message argument to archive_files so that the error message
with a reason for archiving can be included in the tarball.

deluge/common.py

@@ -25,6 +25,8 @@ import subprocess
 import sys
 import tarfile
 import time
+from contextlib import closing
+from io import BytesIO
 
 import pkg_resources
 
@@ -160,7 +162,7 @@ def get_default_download_dir():
     return download_dir
 
 
-def archive_files(arc_name, filepaths):
+def archive_files(arc_name, filepaths, message=None):
     """Compress a list of filepaths into timestamped tarball in config dir.
 
     The archiving config directory is 'archive'.
@@ -197,9 +199,17 @@ def archive_files(arc_name, filepaths):
             log.warning('More than %s tarballs in config archive', max_num_arcs)
 
     try:
-        with tarfile.open(arc_filepath, 'w:' + arc_comp) as tf:
+        with tarfile.open(arc_filepath, 'w:' + arc_comp) as tar:
             for filepath in filepaths:
-                tf.add(filepath, arcname=os.path.basename(filepath))
+                if not os.path.isfile(filepath):
+                    continue
+                tar.add(filepath, arcname=os.path.basename(filepath))
+            if message:
+                with closing(BytesIO(message.encode('utf8'))) as fobj:
+                    tarinfo = tarfile.TarInfo('archive_message.txt')
+                    tarinfo.size = len(fobj.getvalue())
+                    tarinfo.mtime = time.time()
+                    tar.addfile(tarinfo, fileobj=fobj)
     except OSError:
         log.error('Problem occurred archiving filepaths: %s', filepaths)
         return False

deluge/core/torrentmanager.py

@@ -228,14 +228,7 @@ class TorrentManager(component.Component):
     def start(self):
         # Check for old temp file to verify safe shutdown
         if os.path.isfile(self.temp_file):
-            log.warning(
-                'Potential bad shutdown of Deluge detected, archiving torrent state files...'
-            )
-            arc_filepaths = []
-            for filename in ('torrents.fastresume', 'torrents.state'):
-                filepath = os.path.join(self.state_dir, filename)
-                arc_filepaths.extend([filepath, filepath + '.bak'])
-            archive_files('torrents_state', arc_filepaths)
+            self.archive_state('Bad shutdown detected so archiving state files')
             os.remove(self.temp_file)
 
         with open(self.temp_file, 'a'):
@@ -789,6 +782,7 @@ class TorrentManager(component.Component):
         if state.torrents:
             t_state_tmp = TorrentState()
             if dir(state.torrents[0]) != dir(t_state_tmp):
+                self.archive_state('Migration of TorrentState required.')
                 try:
                     for attr in set(dir(t_state_tmp)) - set(dir(state.torrents[0])):
                         for t_state in state.torrents:
@@ -807,21 +801,25 @@ class TorrentManager(component.Component):
 
         """
         torrents_state = os.path.join(self.state_dir, 'torrents.state')
+        state = None
         for filepath in (torrents_state, torrents_state + '.bak'):
             log.info('Loading torrent state: %s', filepath)
+            if not os.path.isfile(filepath):
+                continue
+
             try:
                 with open(filepath, 'rb') as _file:
                     state = pickle.load(_file)
             except (IOError, EOFError, pickle.UnpicklingError) as ex:
-                log.warning('Unable to load %s: %s', filepath, ex)
-                state = None
+                message = 'Unable to load {}: {}'.format(filepath, ex)
+                log.error(message)
+                if not filepath.endswith('.bak'):
+                    self.archive_state(message)
             else:
                 log.info('Successfully loaded %s', filepath)
                 break
 
-        if state is None:
-            state = TorrentManagerState()
-        return state
+        return state if state else TorrentManagerState()
 
     def load_state(self):
         """Load all the torrents from TorrentManager state into session.
@@ -1157,6 +1155,15 @@ class TorrentManager(component.Component):
                 os.close(dirfd)
             return True
 
+    def archive_state(self, message):
+        log.warning(message)
+        arc_filepaths = []
+        for filename in ('torrents.fastresume', 'torrents.state'):
+            filepath = os.path.join(self.state_dir, filename)
+            arc_filepaths.extend([filepath, filepath + '.bak'])
+
+        archive_files('state', arc_filepaths, message=message)
+
     def get_queue_position(self, torrent_id):
         """Get queue position of torrent"""
         return self.torrents[torrent_id].get_queue_position()

deluge/tests/test_common.py

@@ -173,3 +173,30 @@ class CommonTestCase(unittest.TestCase):
                 self.assertTrue(
                     tar_info.name in [os.path.basename(arcf) for arcf in arc_filelist]
                 )
+
+    def test_archive_files_missing(self):
+        """Archive exists even with file not found."""
+        filelist = ['test.torrent', 'deluge.png', 'missing.file']
+        arc_filepath = archive_files(
+            'test-arc', [get_test_data_file(f) for f in filelist]
+        )
+        filelist.remove('missing.file')
+
+        with tarfile.open(arc_filepath, 'r') as tar:
+            self.assertEqual(tar.getnames(), filelist)
+            self.assertTrue(all(tarinfo.isfile() for tarinfo in tar))
+
+    def test_archive_files_message(self):
+        filelist = ['test.torrent', 'deluge.png']
+        arc_filepath = archive_files(
+            'test-arc', [get_test_data_file(f) for f in filelist], message='test'
+        )
+
+        result_files = filelist + ['archive_message.txt']
+        with tarfile.open(arc_filepath, 'r') as tar:
+            self.assertEqual(tar.getnames(), result_files)
+            for tar_info in tar:
+                self.assertTrue(tar_info.isfile())
+                if tar_info.name == 'archive_message.txt':
+                    result = tar.extractfile(tar_info).read().decode()
+                    self.assertEqual(result, 'test')