From 2c4ef9dbb306ebefcb3314b4415c736fc3b263d2 Mon Sep 17 00:00:00 2001
From: Calum Lind <calumlind+deluge@gmail.com>
Date: Tue, 21 May 2013 23:45:26 +0100
Subject: Fixup saving and loading state files

 * All state files have a backup created before saving
 * The backup will now be used if saving or loading fails
 * GTKUI state files stored in new gtkui_state dir and common load/save functions created
 * Detects bad shutdown and archives timestamped state files in separate config directory.
---
 deluge/config.py                  |   4 +-
 deluge/core/authmanager.py        |  66 ++++++++------
 deluge/core/core.py               |  52 ++++++++---
 deluge/core/torrentmanager.py     | 186 +++++++++++++++++++++++++-------------
 deluge/ui/gtkui/common.py         |  52 +++++++++++
 deluge/ui/gtkui/files_tab.py      |  34 ++-----
 deluge/ui/gtkui/gtkui.py          |  20 ++--
 deluge/ui/gtkui/listview.py       |  35 +------
 deluge/ui/gtkui/peers_tab.py      |  29 +-----
 deluge/ui/gtkui/torrentdetails.py |  34 +------
 10 files changed, 283 insertions(+), 229 deletions(-)

diff --git a/deluge/config.py b/deluge/config.py
index ff231691c..8c963c289 100644
--- a/deluge/config.py
+++ b/deluge/config.py
@@ -486,8 +486,8 @@ what is currently in the config and it could not convert the value
 
         # Make a backup of the old config
         try:
-            log.debug("Backing up old config file to %s~", filename)
-            shutil.move(filename, filename + "~")
+            log.debug("Backing up old config file to %s.bak", filename)
+            shutil.move(filename, filename + ".bak")
         except Exception, e:
             log.warning("Unable to backup old config...")
 
diff --git a/deluge/core/authmanager.py b/deluge/core/authmanager.py
index f8cffbb7c..768d9f852 100644
--- a/deluge/core/authmanager.py
+++ b/deluge/core/authmanager.py
@@ -100,7 +100,7 @@ class AuthManager(component.Component):
     def update(self):
         auth_file = configmanager.get_config_dir("auth")
         # Check for auth file and create if necessary
-        if not os.path.exists(auth_file):
+        if not os.path.isfile(auth_file):
             log.info("Authfile not found, recreating it.")
             self.__load_auth_file()
             return
@@ -192,36 +192,40 @@ class AuthManager(component.Component):
         return True
 
     def write_auth_file(self):
-        old_auth_file = configmanager.get_config_dir("auth")
-        new_auth_file = old_auth_file + '.new'
-        bak_auth_file = old_auth_file + '.bak'
-        # Let's first create a backup
-        if os.path.exists(old_auth_file):
-            shutil.copy2(old_auth_file, bak_auth_file)
+        filename = "auth"
+        filepath = os.path.join(configmanager,get_config_dir(), filename)
+        filepath_bak = filepath + ".bak"
 
         try:
-            fd = open(new_auth_file, "w")
-            for account in self.__auth.values():
-                fd.write(
-                    "%(username)s:%(password)s:%(authlevel_int)s\n" %
-                    account.data()
-                )
-            fd.flush()
-            os.fsync(fd.fileno())
-            fd.close()
-            os.rename(new_auth_file, old_auth_file)
-        except:
-            # Something failed, let's restore the previous file
-            if os.path.exists(bak_auth_file):
-                os.rename(bak_auth_file, old_auth_file)
+            if os.path.isfile(filepath):
+                log.info("Creating backup of %s at: %s", filename, filepath_bak)
+                shutil.copy2(filepath, filepath_bak)
+        except IOError as ex:
+            log.error("Unable to backup %s to %s: %s", filepath, filepath_bak, ex)
+        else:
+            log.info("Saving the %s at: %s", filename, filepath)
+            try:
+                with open(filepath, "wb") as _file:
+                    for account in self.__auth.values():
+                        _file.write("%(username)s:%(password)s:%(authlevel_int)s\n" % account.data())
+                    _file.flush()
+                    os.fsync(_file.fileno())
+            except (IOError) as ex:
+                log.error("Unable to save %s: %s", filename, ex)
+                if os.path.isfile(filepath_bak):
+                    log.info("Restoring backup of %s from: %s", filename, filepath_bak)
+                    shutil.move(filepath_bak, filepath)
 
         self.__load_auth_file()
 
     def __load_auth_file(self):
         save_and_reload = False
-        auth_file = configmanager.get_config_dir("auth")
+        filename = "auth"
+        auth_file = configmanager.get_config_dir(filename)
+        auth_file_bak = auth_file + ".bak"
+
         # Check for auth file and create if necessary
-        if not os.path.exists(auth_file):
+        if not os.path.isfile(auth_file):
             create_localclient_account()
             return self.__load_auth_file()
 
@@ -232,10 +236,20 @@ class AuthManager(component.Component):
             # File didn't change, no need for re-parsing's
             return
 
-        # Load the auth file into a dictionary: {username: Account(...)}
-        f = open(auth_file, "r").readlines()
+        for _filepath in (auth_file, auth_file_bak):
+            log.info("Opening %s for load: %s", filename, _filepath)
+            try:
+                with open(_filepath, "rb") as _file:
+                    file_data = _file.readlines()
+            except (IOError), ex:
+                log.warning("Unable to load %s: %s", _filepath, ex)
+                file_data = []
+            else:
+                log.info("Successfully loaded %s: %s", filename, _filepath)
+                break
 
-        for line in f:
+        # Load the auth file into a dictionary: {username: Account(...)}
+        for line in file_data:
             line = line.strip()
             if line.startswith("#") or not line:
                 # This line is a comment or empty
diff --git a/deluge/core/core.py b/deluge/core/core.py
index f971565ba..6b6757d34 100644
--- a/deluge/core/core.py
+++ b/deluge/core/core.py
@@ -38,6 +38,7 @@ from deluge._libtorrent import lt
 
 import os
 import glob
+import shutil
 import base64
 import logging
 import threading
@@ -50,7 +51,7 @@ import twisted.web.error
 from deluge.httpdownloader import download_file
 from deluge import path_chooser_common
 
-import deluge.configmanager
+from deluge.configmanager import ConfigManager, get_config_dir
 import deluge.common
 import deluge.component as component
 from deluge.event import *
@@ -120,7 +121,7 @@ class Core(component.Component):
         self.new_release = None
 
         # Get the core config
-        self.config = deluge.configmanager.ConfigManager("core.conf")
+        self.config = ConfigManager("core.conf")
         self.config.save()
 
         # If there was an interface value from the command line, use it, but
@@ -153,19 +154,46 @@ class Core(component.Component):
 
     def __save_session_state(self):
         """Saves the libtorrent session state"""
+        filename = "session.state"
+        filepath = get_config_dir(filename)
+        filepath_bak = filepath + ".bak"
+
         try:
-            session_state = deluge.configmanager.get_config_dir("session.state")
-            open(session_state, "wb").write(lt.bencode(self.session.save_state()))
-        except Exception, e:
-            log.warning("Failed to save lt state: %s", e)
+            if os.path.isfile(filepath):
+                log.info("Creating backup of %s at: %s", filename, filepath_bak)
+                shutil.copy2(filepath, filepath_bak)
+        except IOError as ex:
+            log.error("Unable to backup %s to %s: %s", filepath, filepath_bak, ex)
+        else:
+            log.info("Saving the %s at: %s", filename, filepath)
+            try:
+                with open(filepath, "wb") as _file:
+                    _file.write(lt.bencode(self.session.save_state()))
+                    _file.flush()
+                    os.fsync(_file.fileno())
+            except (IOError, EOFError) as ex:
+                log.error("Unable to save %s: %s", filename, ex)
+                if os.path.isfile(filepath_bak):
+                    log.info("Restoring backup of %s from: %s", filename, filepath_bak)
+                    shutil.move(filepath_bak, filepath)
 
     def __load_session_state(self):
         """Loads the libtorrent session state"""
-        try:
-            session_state = deluge.configmanager.get_config_dir("session.state")
-            self.session.load_state(lt.bdecode(open(session_state, "rb").read()))
-        except Exception, e:
-            log.warning("Failed to load lt state: %s", e)
+        filename = "session.state"
+        filepath = get_config_dir(filename)
+        filepath_bak = filepath + ".bak"
+
+        for _filepath in (filepath, filepath_bak):
+            log.info("Opening %s for load: %s", filename, _filepath)
+            try:
+                with open(_filepath, "rb") as _file:
+                    state = lt.bdecode(_file.read())
+            except (IOError, EOFError, RuntimeError), ex:
+                log.warning("Unable to load %s: %s", _filepath, ex)
+            else:
+                log.info("Successfully loaded %s: %s", filename, _filepath)
+                self.session.load_state(state)
+                return
 
     def get_new_release(self):
         log.debug("get_new_release")
@@ -679,7 +707,7 @@ class Core(component.Component):
             log.exception(e)
             return
 
-        f = open(os.path.join(deluge.configmanager.get_config_dir(), "plugins", filename), "wb")
+        f = open(os.path.join(get_config_dir(), "plugins", filename), "wb")
         f.write(filedump)
         f.close()
         component.get("CorePluginManager").scan_for_plugins()
diff --git a/deluge/core/torrentmanager.py b/deluge/core/torrentmanager.py
index 884e58867..a2f2a4264 100644
--- a/deluge/core/torrentmanager.py
+++ b/deluge/core/torrentmanager.py
@@ -144,8 +144,9 @@ class TorrentManager(component.Component):
         self.config = ConfigManager("core.conf")
 
         # Make sure the state folder has been created
-        if not os.path.exists(os.path.join(get_config_dir(), "state")):
-            os.makedirs(os.path.join(get_config_dir(), "state"))
+        self.state_dir = os.path.join(get_config_dir(), "state")
+        if not os.path.exists(self.state_dir):
+            os.makedirs(self.state_dir)
 
         # Create the torrents dict { torrent_id: Torrent }
         self.torrents = {}
@@ -212,6 +213,34 @@ class TorrentManager(component.Component):
         # Get the pluginmanager reference
         self.plugins = component.get("CorePluginManager")
 
+        # Check for temp file
+        self.temp_file = os.path.join(self.state_dir, ".safe_state_check")
+        if os.path.isfile(self.temp_file):
+            def archive_file(filename):
+                import datetime
+                filepath = os.path.join(self.state_dir, filename)
+                filepath_bak = state_filepath + ".bak"
+                archive_dir = os.path.join(get_config_dir(), "archive")
+                if not os.path.exists(archive_dir):
+                    os.makedirs(archive_dir)
+
+                for _filepath in (filepath, filepath_bak):
+                    timestamp = datetime.datetime.now().replace(microsecond=0).isoformat().replace(':', '-')
+                    archive_filepath = os.path.join(archive_dir, filename + "-" + timestamp)
+                    try:
+                        shutil.copy2(_filepath, archive_filepath)
+                    except IOError:
+                        log.error("Unable to archive: %s", filename)
+                    else:
+                        log.info("Archive of %s successful: %s", filename, archive_filepath)
+
+            log.warning("Potential bad shutdown of Deluge detected, archiving torrent state files...")
+            archive_file("torrents.state")
+            archive_file("torrents.fastresume")
+        else:
+            with file(self.temp_file, 'a'):
+                os.utime(self.temp_file, None)
+
         # Run the old state upgrader before loading state
         deluge.core.oldstateupgrader.OldStateUpgrader()
 
@@ -246,7 +275,13 @@ class TorrentManager(component.Component):
             # Stop the status cleanup LoopingCall here
             self.torrents[key].prev_status_cleanup_loop.stop()
 
-        return self.save_resume_data(self.torrents.keys())
+        def remove_temp_file(result):
+            if result and os.path.isfile(self.temp_file):
+                os.remove(self.temp_file)
+
+        d = self.save_resume_data(self.torrents.keys())
+        d.addCallback(remove_temp_file)
+        return d
 
     def update(self):
         for torrent_id, torrent in self.torrents.items():
@@ -301,8 +336,7 @@ class TorrentManager(component.Component):
         """Returns an entry with the resume data or None"""
         fastresume = ""
         try:
-            _file = open(os.path.join(get_config_dir(), "state",
-                                      torrent_id + ".fastresume"), "rb")
+            _file = open(os.path.join(self.state_dir, torrent_id + ".fastresume"), "rb")
             fastresume = _file.read()
             _file.close()
         except IOError, e:
@@ -312,8 +346,7 @@ class TorrentManager(component.Component):
 
     def legacy_delete_resume_data(self, torrent_id):
         """Deletes the .fastresume file"""
-        path = os.path.join(get_config_dir(), "state",
-                            torrent_id + ".fastresume")
+        path = os.path.join(self.state_dir, torrent_id + ".fastresume")
         log.debug("Deleting fastresume file: %s", path)
         try:
             os.remove(path)
@@ -501,9 +534,7 @@ class TorrentManager(component.Component):
         # Write the .torrent file to the state directory
         if filedump:
             try:
-                save_file = open(os.path.join(get_config_dir(), "state",
-                        torrent.torrent_id + ".torrent"),
-                        "wb")
+                save_file = open(os.path.join(self.state_dir, torrent.torrent_id + ".torrent"), "wb")
                 save_file.write(filedump)
                 save_file.close()
             except IOError, e:
@@ -546,10 +577,7 @@ class TorrentManager(component.Component):
         # Get the torrent data from the torrent file
         try:
             log.debug("Attempting to open %s for add.", torrent_id)
-            _file = open(
-                os.path.join(
-                    get_config_dir(), "state", torrent_id + ".torrent"),
-                        "rb")
+            _file = open(os.path.join(self.state_dir, torrent_id + ".torrent"), "rb")
             filedump = lt.bdecode(_file.read())
             _file.close()
         except (IOError, RuntimeError), e:
@@ -636,16 +664,24 @@ class TorrentManager(component.Component):
 
     def load_state(self):
         """Load the state of the TorrentManager from the torrents.state file"""
-        state = TorrentManagerState()
+        filename = "torrents.state"
+        filepath = os.path.join(self.state_dir, filename)
+        filepath_bak = filepath + ".bak"
 
-        try:
-            log.debug("Opening torrent state file for load.")
-            state_file = open(
-                os.path.join(get_config_dir(), "state", "torrents.state"), "rb")
-            state = cPickle.load(state_file)
-            state_file.close()
-        except (EOFError, IOError, Exception, cPickle.UnpicklingError), e:
-            log.warning("Unable to load state file: %s", e)
+        for _filepath in (filepath, filepath_bak):
+            log.info("Opening %s for load: %s", filename, _filepath)
+            try:
+                with open(_filepath, "rb") as _file:
+                    state = cPickle.load(_file)
+            except (IOError, EOFError, cPickle.UnpicklingError), ex:
+                log.warning("Unable to load %s: %s", _filepath, ex)
+                state = None
+            else:
+                log.info("Successfully loaded %s: %s", filename, _filepath)
+                break
+
+        if state is None:
+            state = TorrentManagerState()
 
         # Try to use an old state
         try:
@@ -727,28 +763,29 @@ class TorrentManager(component.Component):
             )
             state.torrents.append(torrent_state)
 
-        # Pickle the TorrentManagerState object
-        try:
-            log.debug("Saving torrent state file.")
-            state_file = open(os.path.join(get_config_dir(),
-                              "state", "torrents.state.new"), "wb")
-            cPickle.dump(state, state_file)
-            state_file.flush()
-            os.fsync(state_file.fileno())
-            state_file.close()
-        except IOError, e:
-            log.warning("Unable to save state file: %s", e)
-            return True
+        filename = "torrents.state"
+        filepath = os.path.join(self.state_dir, filename)
+        filepath_bak = filepath + ".bak"
 
-        # We have to move the 'torrents.state.new' file to 'torrents.state'
         try:
-            shutil.move(
-                os.path.join(get_config_dir(), "state", "torrents.state.new"),
-                os.path.join(get_config_dir(), "state", "torrents.state"))
-        except IOError:
-            log.warning("Unable to save state file.")
-            return True
-
+            if os.path.isfile(filepath):
+                log.info("Creating backup of %s at: %s", filename, filepath_bak)
+                shutil.copy2(filepath, filepath_bak)
+        except IOError as ex:
+            log.error("Unable to backup %s to %s: %s", filepath, filepath_bak, ex)
+        else:
+            log.info("Saving the %s at: %s", filename, filepath)
+            try:
+                with open(filepath, "wb") as _file:
+                    # Pickle the TorrentManagerState object
+                    cPickle.dump(state, _file)
+                    _file.flush()
+                    os.fsync(_file.fileno())
+            except (IOError, cPickle.PicklingError) as ex:
+                log.error("Unable to save %s: %s", filename, ex)
+                if os.path.isfile(filepath_bak):
+                    log.info("Restoring backup of %s from: %s", filename, filepath_bak)
+                    shutil.move(filepath_bak, filepath)
         # We return True so that the timer thread will continue
         return True
 
@@ -760,7 +797,6 @@ class TorrentManager(component.Component):
         :returns: A Deferred whose callback will be invoked when save is complete
         :rtype: twisted.internet.defer.Deferred
         """
-
         if torrent_ids is None:
             torrent_ids = (t[0] for t in self.torrents.iteritems() if t[1].handle.need_save_resume_data())
 
@@ -779,43 +815,63 @@ class TorrentManager(component.Component):
 
         def on_all_resume_data_finished(result):
             if result:
-                self.save_resume_data_file()
+                if self.save_resume_data_file():
+                    return True
 
         return DeferredList(deferreds).addBoth(on_all_resume_data_finished)
 
     def load_resume_data_file(self):
-        resume_data = {}
-        try:
-            log.debug("Opening torrents fastresume file for load.")
-            fastresume_file = open(os.path.join(get_config_dir(), "state",
-                                                "torrents.fastresume"), "rb")
-            resume_data = lt.bdecode(fastresume_file.read())
-            fastresume_file.close()
-        except (EOFError, IOError, Exception), e:
-            log.warning("Unable to load fastresume file: %s", e)
+        filename = "torrents.fastresume"
+        filepath = os.path.join(self.state_dir, filename)
+        filepath_bak = filepath + ".bak"
+        old_data_filepath = os.path.join(get_config_dir(), filename)
 
+        for _filepath in (filepath, filepath_bak, old_data_filepath):
+            log.info("Opening %s for load: %s", filename, _filepath)
+            try:
+                with open(_filepath, "rb") as _file:
+                    resume_data = lt.bdecode(_file.read())
+            except (IOError, EOFError, RuntimeError), ex:
+                log.warning("Unable to load %s: %s", _filepath, ex)
+                resume_data = None
+            else:
+                log.info("Successfully loaded %s: %s", filename, _filepath)
+                break
         # If the libtorrent bdecode doesn't happen properly, it will return None
         # so we need to make sure we return a {}
         if resume_data is None:
             return {}
-
-        return resume_data
+        else:
+            return resume_data
 
     def save_resume_data_file(self):
         """
         Saves the resume data file with the contents of self.resume_data.
         """
-        path = os.path.join(get_config_dir(), "state", "torrents.fastresume")
+        filename = "torrents.fastresume"
+        filepath = os.path.join(self.state_dir, filename)
+        filepath_bak = filepath + ".bak"
 
         try:
-            log.debug("Saving fastresume file: %s", path)
-            fastresume_file = open(path, "wb")
-            fastresume_file.write(lt.bencode(self.resume_data))
-            fastresume_file.flush()
-            os.fsync(fastresume_file.fileno())
-            fastresume_file.close()
-        except IOError:
-            log.warning("Error trying to save fastresume file")
+            if os.path.isfile(filepath):
+                log.info("Creating backup of %s at: %s", filename, filepath_bak)
+                shutil.copy2(filepath, filepath_bak)
+        except IOError as ex:
+            log.error("Unable to backup %s to %s: %s", filepath, filepath_bak, ex)
+        else:
+            log.info("Saving the %s at: %s", filename, filepath)
+            try:
+                with open(filepath, "wb") as _file:
+                    _file.write(lt.bencode(self.session.save_state()))
+                    _file.flush()
+                    os.fsync(_file.fileno())
+            except (IOError, EOFError) as ex:
+                log.error("Unable to save %s: %s", filename, ex)
+                if os.path.isfile(filepath_bak):
+                    log.info("Restoring backup of %s from: %s", filename, filepath_bak)
+                    shutil.move(filepath_bak, filepath)
+            else:
+                return True
 
     def get_queue_position(self, torrent_id):
         """Get queue position of torrent"""
diff --git a/deluge/ui/gtkui/common.py b/deluge/ui/gtkui/common.py
index 088223803..1c31453ba 100644
--- a/deluge/ui/gtkui/common.py
+++ b/deluge/ui/gtkui/common.py
@@ -41,6 +41,8 @@ import pygtk
 pygtk.require('2.0')
 import gtk
 import logging
+import cPickle
+import shutil
 
 import deluge.component as component
 import deluge.common
@@ -264,3 +266,53 @@ def associate_magnet_links(overwrite=False):
                     log.error("Unable to register Deluge as default magnet uri handler.")
                     return False
     return False
+
+def save_pickled_state_file(filename, state):
+    """Save a file in the config directory and creates a backup
+    filename: Filename to be saved to config
+    state: The data to be pickled and written to file
+    """
+    from deluge.configmanager import get_config_dir
+    filepath = os.path.join(get_config_dir(), "gtkui_state", filename)
+    filepath_bak = filepath + ".bak"
+
+    try:
+        if os.path.isfile(filepath):
+            log.info("Creating backup of %s at: %s", filename, filepath_bak)
+            shutil.copy2(filepath, filepath_bak)
+    except IOError as ex:
+        log.error("Unable to backup %s to %s: %s", filepath, filepath_bak, ex)
+    else:
+        log.info("Saving the %s at: %s", filename, filepath)
+        try:
+            with open(filepath, "wb") as _file:
+                # Pickle the state object
+                cPickle.dump(state, _file)
+                _file.flush()
+                os.fsync(_file.fileno())
+        except (IOError, EOFError, cPickle.PicklingError) as ex:
+            log.error("Unable to save %s: %s", filename, ex)
+            if os.path.isfile(filepath_bak):
+                log.info("Restoring backup of %s from: %s", filename, filepath_bak)
+                shutil.move(filepath_bak, filepath)
+
+def load_pickled_state_file(filename):
+    """Loads a file from the config directory, attempting backup if original fails to load.
+    filename: Filename to be loaded from config
+    returns unpickled state
+    """
+    from deluge.configmanager import get_config_dir
+    filepath = os.path.join(get_config_dir(), "gtkui_state", filename)
+    filepath_bak = filepath + ".bak"
+    old_data_filepath = os.path.join(get_config_dir(), filename)
+
+    for _filepath in (filepath, filepath_bak, old_data_filepath):
+        log.info("Opening %s for load: %s", filename, _filepath)
+        try:
+            with open(_filepath, "rb") as _file:
+                state = cPickle.load(_file)
+        except (IOError, cPickle.UnpicklingError), ex:
+            log.warning("Unable to load %s: %s", _filepath, ex)
+        else:
+            log.info("Successfully loaded %s: %s", filename, _filepath)
+            return state
diff --git a/deluge/ui/gtkui/files_tab.py b/deluge/ui/gtkui/files_tab.py
index 82f8ec573..5fa36c48a 100644
--- a/deluge/ui/gtkui/files_tab.py
+++ b/deluge/ui/gtkui/files_tab.py
@@ -38,15 +38,14 @@ import gtk
 import gtk.gdk
 import gobject
 import os.path
-import cPickle
 import logging
+import cPickle
 
 from deluge.ui.gtkui.torrentdetails import Tab
 from deluge.ui.client import client
-import deluge.configmanager
 import deluge.component as component
 import deluge.common
-import common
+from deluge.ui.gtkui.common import reparent_iter, save_pickled_state_file, load_pickled_state_file
 
 log = logging.getLogger(__name__)
 
@@ -242,7 +241,6 @@ class FilesTab(Tab):
             getattr(widget, attr)()
 
     def save_state(self):
-        filename = "files_tab.state"
         # Get the current sort order of the view
         column_id, sort_order = self.treestore.get_sort_column_id()
 
@@ -259,30 +257,10 @@ class FilesTab(Tab):
                 "width": column.get_width()
             }
 
-        # Get the config location for saving the state file
-        config_location = deluge.configmanager.get_config_dir()
-
-        try:
-            log.debug("Saving FilesTab state file: %s", filename)
-            state_file = open(os.path.join(config_location, filename), "wb")
-            cPickle.dump(state, state_file)
-            state_file.close()
-        except IOError, e:
-            log.warning("Unable to save state file: %s", e)
+        save_pickled_state_file("files_tab.state", state)
 
     def load_state(self):
-        filename = "files_tab.state"
-        # Get the config location for loading the state file
-        config_location = deluge.configmanager.get_config_dir()
-        state = None
-
-        try:
-            log.debug("Loading FilesTab state file: %s", filename)
-            state_file = open(os.path.join(config_location, filename), "rb")
-            state = cPickle.load(state_file)
-            state_file.close()
-        except (EOFError, IOError, AttributeError, cPickle.UnpicklingError), e:
-            log.warning("Unable to load state file: %s", e)
+        state = load_pickled_state_file("files_tabs.state")
 
         if state == None:
             return
@@ -807,14 +785,14 @@ class FilesTab(Tab):
                 return
             if new_folder_iter:
                 # This means that a folder by this name already exists
-                common.reparent_iter(self.treestore, self.treestore.iter_children(old_folder_iter), new_folder_iter)
+                reparent_iter(self.treestore, self.treestore.iter_children(old_folder_iter), new_folder_iter)
             else:
                 parent = old_folder_iter_parent
                 for ns in new_split[:-1]:
                     parent = self.treestore.append(parent, [ns + "/", 0, "", 0, 0, -1, gtk.STOCK_DIRECTORY])
 
                 self.treestore[old_folder_iter][0] = new_split[-1] + "/"
-                common.reparent_iter(self.treestore, old_folder_iter, parent)
+                reparent_iter(self.treestore, old_folder_iter, parent)
 
             # We need to check if the old_folder_iter_parent no longer has children
             # and if so, we delete it
diff --git a/deluge/ui/gtkui/gtkui.py b/deluge/ui/gtkui/gtkui.py
index 28b892d58..f4ed2be9f 100644
--- a/deluge/ui/gtkui/gtkui.py
+++ b/deluge/ui/gtkui/gtkui.py
@@ -40,6 +40,7 @@ gobject.set_prgname("deluge")
 from twisted.internet import gtk2reactor
 reactor = gtk2reactor.install()
 
+import os
 import gtk
 import sys
 import logging
@@ -71,14 +72,13 @@ from queuedtorrents import QueuedTorrents
 from addtorrentdialog import AddTorrentDialog
 from deluge.ui.sessionproxy import SessionProxy
 import dialogs
-import common
-
-import deluge.configmanager
+from deluge.ui.gtkui.common import associate_magnet_links
+from deluge.configmanager import ConfigManager, get_config_dir
 import deluge.common
 import deluge.error
-
 from deluge.ui.ui import _UI
 
+
 class Gtk(_UI):
 
     help = """Starts the Deluge GTK+ interface"""
@@ -90,6 +90,7 @@ class Gtk(_UI):
         super(Gtk, self).start()
         GtkUI(self.args)
 
+
 def start():
     Gtk().start()
 
@@ -152,6 +153,7 @@ DEFAULT_PREFS = {
     "focus_main_window_on_add": True,
 }
 
+
 class GtkUI(object):
     def __init__(self, args):
         self.daemon_bps = (0,0,0)
@@ -192,10 +194,14 @@ class GtkUI(object):
 
         # Attempt to register a magnet URI handler with gconf, but do not overwrite
         # if already set by another program.
-        common.associate_magnet_links(False)
+        associate_magnet_links(False)
 
         # Make sure gtkui.conf has at least the defaults set
-        self.config = deluge.configmanager.ConfigManager("gtkui.conf", DEFAULT_PREFS)
+        self.config = ConfigManager("gtkui.conf", DEFAULT_PREFS)
+
+        # Make sure the gtkui state folder has been created
+        if not os.path.exists(os.path.join(get_config_dir(), "gtkui_state")):
+            os.makedirs(os.path.join(get_config_dir(), "gtkui_state"))
 
         # We need to check on exit if it was started in classic mode to ensure we
         # shutdown the daemon.
@@ -370,7 +376,7 @@ Please see the details below for more information."), details=traceback.format_e
                         if self.config["autostart_localhost"] and host in ("localhost", "127.0.0.1"):
                             log.debug("Autostarting localhost:%s", host)
                             try_connect = client.start_daemon(
-                                port, deluge.configmanager.get_config_dir()
+                                port, get_config_dir()
                             )
                             log.debug("Localhost started: %s", try_connect)
                             if not try_connect:
diff --git a/deluge/ui/gtkui/listview.py b/deluge/ui/gtkui/listview.py
index 07af28b7e..07f45d1a6 100644
--- a/deluge/ui/gtkui/listview.py
+++ b/deluge/ui/gtkui/listview.py
@@ -34,19 +34,13 @@
 #
 #
 
-
-import cPickle
-import os.path
 import logging
-
 import pygtk
 pygtk.require('2.0')
 import gtk
-import gettext
 
-from deluge.configmanager import ConfigManager
-import deluge.configmanager
 import deluge.common
+from deluge.ui.gtkui.common import save_pickled_state_file, load_pickled_state_file
 
 from gobject import signal_new, SIGNAL_RUN_LAST, TYPE_NONE
 from gtk import gdk
@@ -333,34 +327,11 @@ class ListView:
             state.append(self.create_column_state(column, counter))
 
         state += self.removed_columns_state
-
-        # Get the config location for saving the state file
-        config_location = deluge.configmanager.get_config_dir()
-
-        try:
-            log.debug("Saving ListView state file: %s", filename)
-            state_file = open(os.path.join(config_location, filename), "wb")
-            cPickle.dump(state, state_file)
-            state_file.close()
-        except IOError, e:
-            log.warning("Unable to save state file: %s", e)
+        save_pickled_state_file(filename, state)
 
     def load_state(self, filename):
         """Load the listview state from filename."""
-        # Get the config location for loading the state file
-        config_location = deluge.configmanager.get_config_dir()
-        state = None
-
-        try:
-            log.debug("Loading ListView state file: %s", filename)
-            state_file = open(os.path.join(config_location, filename), "rb")
-            state = cPickle.load(state_file)
-            state_file.close()
-        except (EOFError, IOError, cPickle.UnpicklingError), e:
-            log.warning("Unable to load state file: %s", e)
-
-        # Keep the state in self.state so we can access it as we add new columns
-        self.state = state
+        self.state = load_pickled_state_file(filename)
 
     def set_treeview(self, treeview_widget):
         """Set the treeview widget that this listview uses."""
diff --git a/deluge/ui/gtkui/peers_tab.py b/deluge/ui/gtkui/peers_tab.py
index e3adde3ac..5efbaa385 100644
--- a/deluge/ui/gtkui/peers_tab.py
+++ b/deluge/ui/gtkui/peers_tab.py
@@ -36,16 +36,15 @@
 import gtk
 import logging
 import os.path
-import cPickle
 from itertools import izip
 
 from deluge.ui.client import client
-import deluge.configmanager
 import deluge.component as component
 import deluge.common
 from deluge.ui.gtkui.listview import cell_data_speed as cell_data_speed
 from deluge.ui.gtkui.torrentdetails import Tab
 from deluge.ui.countries import COUNTRIES
+from deluge.ui.gtkui.common import save_pickled_state_file, load_pickled_state_file
 
 log = logging.getLogger(__name__)
 
@@ -171,7 +170,6 @@ class PeersTab(Tab):
         self.torrent_id = None
 
     def save_state(self):
-        filename = "peers_tab.state"
         # Get the current sort order of the view
         column_id, sort_order = self.liststore.get_sort_column_id()
 
@@ -187,31 +185,10 @@ class PeersTab(Tab):
                 "position": index,
                 "width": column.get_width()
             }
-
-        # Get the config location for saving the state file
-        config_location = deluge.configmanager.get_config_dir()
-
-        try:
-            log.debug("Saving FilesTab state file: %s", filename)
-            state_file = open(os.path.join(config_location, filename), "wb")
-            cPickle.dump(state, state_file)
-            state_file.close()
-        except IOError, e:
-            log.warning("Unable to save state file: %s", e)
+        save_pickled_state_file("peers_tab.state", state)
 
     def load_state(self):
-        filename = "peers_tab.state"
-        # Get the config location for loading the state file
-        config_location = deluge.configmanager.get_config_dir()
-        state = None
-
-        try:
-            log.debug("Loading PeersTab state file: %s", filename)
-            state_file = open(os.path.join(config_location, filename), "rb")
-            state = cPickle.load(state_file)
-            state_file.close()
-        except (EOFError, IOError, AttributeError, cPickle.UnpicklingError), e:
-            log.warning("Unable to load state file: %s", e)
+        state = load_pickled_state_file("peers_tabs.state")
 
         if state == None:
             return
diff --git a/deluge/ui/gtkui/torrentdetails.py b/deluge/ui/gtkui/torrentdetails.py
index 2d836c6eb..3de609dd7 100644
--- a/deluge/ui/gtkui/torrentdetails.py
+++ b/deluge/ui/gtkui/torrentdetails.py
@@ -37,15 +37,11 @@
 """The torrent details component shows info about the selected torrent."""
 
 import gtk
-import os
-import os.path
-import cPickle
 import logging
 
 import deluge.component as component
 from deluge.ui.client import client
-from deluge.configmanager import ConfigManager
-import deluge.configmanager
+from deluge.ui.gtkui.common import save_pickled_state_file, load_pickled_state_file
 
 log = logging.getLogger(__name__)
 
@@ -411,8 +407,6 @@ class TorrentDetails(component.Component):
 
     def save_state(self):
         """We save the state, which is basically the tab_index list"""
-        filename = "tabs.state"
-
         #Update the visiblity status of all tabs
         #Leave tabs we dont know anything about it the state as they
         #might come from a plugin
@@ -423,29 +417,7 @@ class TorrentDetails(component.Component):
                 log.debug("Set to %s %d" % self.state[i])
         state = self.state
 
-        # Get the config location for saving the state file
-        config_location = deluge.configmanager.get_config_dir()
-
-        try:
-            log.debug("Saving TorrentDetails state file: %s", filename)
-            state_file = open(os.path.join(config_location, filename), "wb")
-            cPickle.dump(state, state_file)
-            state_file.close()
-        except IOError, e:
-            log.warning("Unable to save state file: %s", e)
+        save_pickled_state_file("tabs.state", state)
 
     def load_state(self):
-        filename = "tabs.state"
-        # Get the config location for loading the state file
-        config_location = deluge.configmanager.get_config_dir()
-        state = None
-
-        try:
-            log.debug("Loading TorrentDetails state file: %s", filename)
-            state_file = open(os.path.join(config_location, filename), "rb")
-            state = cPickle.load(state_file)
-            state_file.close()
-        except (EOFError, IOError, cPickle.UnpicklingError), e:
-            log.warning("Unable to load state file: %s", e)
-
-        return state
+        return load_pickled_state_file("tabs.state")
-- 
cgit