fix prefs maybe replaced with the default value when read and truncat…

…e at the same time

Open prefs.bin via 'wb' will truncate file without lock protection.
So read prefs.bin and truncate prefs.bin at the same time,
the preferences maybe replaced with the default value.
Add a prefs.lock file to guarantee the atomicity of save and load operation.

The patch can not solve phantom read when targetcli command concurrency.
That is, if change preference at same time as execute another targetcli command,
next ConfigShell will load old prefs.bin file before previous ConfigShell save it.
To fix this problem, targetcli-fb:scripts\targetcli must be modified.

Signed-off-by: Zou Mingzhe <[email protected]>

configshell/prefs.py

@@ -33,7 +33,7 @@ class Prefs(object):
     autosave = False
     __borg_state = {}
 
-    def __init__(self, filename=None):
+    def __init__(self, filename=None, lockfile=None):
         '''
         Instanciates the ConfigShell preferences object.
         @param filename: File to store the preferencces to.
@@ -42,6 +42,11 @@ def __init__(self, filename=None):
         self.__dict__ = self.__borg_state
         if filename is not None:
             self.filename = filename
+        if lockfile is None:
+            prefs_dir = os.getenv("TARGETCLI_HOME", '~/.targetcli')
+            prefs_dir = os.path.expanduser(prefs_dir)
+            lockfile = os.path.join(prefs_dir, 'prefs.lock')
+        self.lockfile = lockfile
 
     def __getitem__(self, key):
         '''
@@ -129,14 +134,20 @@ def save(self, filename=None):
             filename = self.filename
 
         if filename is not None:
-            fsock = open(filename, 'wb')
+            flk = open(self.lockfile, 'wb')
+            fcntl.flock(flk, fcntl.LOCK_EX)
             try:
-                fcntl.flock(fsock, fcntl.LOCK_EX)
-                six.moves.cPickle.dump(self._prefs, fsock, 2)
-                fsock.flush()
+                fpref = open(filename, 'wb')
+                try:
+                    fcntl.flock(fpref, fcntl.LOCK_EX)
+                    six.moves.cPickle.dump(self._prefs, fpref, 2)
+                    fpref.flush()
+                finally:
+                    fcntl.flock(fpref, fcntl.LOCK_UN)
+                    fpref.close()
             finally:
-                fcntl.flock(fsock, fcntl.LOCK_UN)
-                fsock.close()
+                fcntl.flock(flk, fcntl.LOCK_UN)
+                flk.close()
 
     def load(self, filename=None):
         '''
@@ -147,11 +158,16 @@ def load(self, filename=None):
             filename = self.filename
 
         if filename is not None and os.path.isfile(filename):
-            fsock = open(filename, 'rb')
+            flk = open(self.lockfile, 'wb')
+            fcntl.flock(flk, fcntl.LOCK_SH)
             try:
-                fcntl.flock(fsock, fcntl.LOCK_SH)
-                if os.path.getsize(filename) > 0:
-                    self._prefs = six.moves.cPickle.load(fsock)
+                fpref = open(filename, 'rb')
+                try:
+                    fcntl.flock(fpref, fcntl.LOCK_SH)
+                    self._prefs = six.moves.cPickle.load(fpref)
+                finally:
+                    fcntl.flock(fpref, fcntl.LOCK_UN)
+                    fpref.close()
             finally:
-                fcntl.flock(fsock, fcntl.LOCK_UN)
-                fsock.close()
+                fcntl.flock(flk, fcntl.LOCK_UN)
+                flk.close()

configshell/shell.py

@@ -139,7 +139,8 @@ def __init__(self, preferences_dir=None):
             if not os.path.exists(preferences_dir):
                 os.makedirs(preferences_dir)
             self._prefs_file = preferences_dir + '/prefs.bin'
-            self.prefs = prefs.Prefs(self._prefs_file)
+            self._prefs_lock = preferences_dir + '/prefs.lock'
+            self.prefs = prefs.Prefs(self._prefs_file, self._prefs_lock)
             self._cmd_history = preferences_dir + '/history.txt'
             self._save_history = True
             if not os.path.isfile(self._cmd_history):