Merge lp:~bladernr/checkbox/1155619-fix-exit-io-error into lp:checkbox

[Jeff Lane ]

First: Fixed the I/O Available message. We weren't properly releasing the lock, so I added bits to use the glock release function at shutdown.

This exposed a different issue where the input and output fifo file descriptors were coming back as invalid on close, because for some reason, those FDs were closed out before the close() function of fifo.py was called... so I just wrapped those to catch the exception and comment on it.

This all occurs RIGHT at shutdown so it's all just garbage collection.

[lp:~bladernr/checkbox/1155619-fix-exit-io-error]

2064. By Jeff Lane  on 2013-04-19

rebased to make tarmac happy

[Daniel Manrique (roadmr)]

+1, thanks! Great work tracking this down.

review: Approve

=== modified file 'checkbox/lib/fifo.py'
--- checkbox/lib/fifo.py	2012-06-09 15:50:39 +0000
+++ checkbox/lib/fifo.py	2013-04-19 21:23:27 +0000
@@ -18,6 +18,7 @@
 #
 import os
 import struct
+import logging
 
 from checkbox.contrib.bpickle import dumps, loads
 from checkbox.lib.selector import Selector, SelectorIO
@@ -36,7 +37,11 @@
         self.close()
 
     def close(self):
-        os.close(self.fileno)
+        try:
+            os.close(self.fileno)
+        except OSError as e:
+            logging.warning("Problem closing the fifo")
+            logging.warning(e)
 
     def wait_for(self, operation):
         if self._timeout is not None:
 
=== modified file 'debian/changelog'
--- debian/changelog	2013-04-19 20:16:22 +0000
+++ debian/changelog	2013-04-19 21:23:27 +0000
@@ -10,6 +10,10 @@
   * jobs/led.txt.in: Modified Jobs: led/power, led/wlan, led/wlan-disabled. New
     jobs: led/power-blink-suspend, led/suspend, led/mute
     jobs/keys.txt.in: modified keys/wireless
+  * plugins/lock_prompt.py: added a lock release to cleanly clear lock on stop.
+    plugins/persist_prompt.py: promoted save to run before lock release.
+    checkbox/lib/fifo.py: trap OSError exception at close when the input/output
+    fifo fds disappear before fifo.close() can get to them. (LP: #115561)
 
   [ Brendan Donegan ]
   * scripts/sources_test - modified script so that it takes sources list
 
=== modified file 'plugins/lock_prompt.py'
--- plugins/lock_prompt.py	2011-02-15 19:43:24 +0000
+++ plugins/lock_prompt.py	2013-04-19 21:23:27 +0000
@@ -20,7 +20,7 @@
 import fcntl
 import posixpath
 import signal
-
+import logging
 from time import time
 
 from gettext import gettext as _
@@ -40,21 +40,24 @@
 
     # Timeout after which to show an error prompt.
     timeout = Int(default=0)
+    logger = logging.getLogger()
 
     def register(self, manager):
         super(LockPrompt, self).register(manager)
 
         self._lock = None
+        self._fd = None
 
         self._manager.reactor.call_on(
             "prompt-begin", self.prompt_begin, -1000)
+        self._manager.reactor.call_on("stop", self.release, 1000)
 
     def prompt_begin(self, interface):
         directory = posixpath.dirname(self.filename)
         safe_make_directory(directory)
 
         # Try to lock the process
-        self._lock = GlobalLock(self.filename)
+        self._lock = GlobalLock(self.filename, logger=self.logger)
         try:
             self._lock.acquire()
         except LockAlreadyAcquired:
@@ -67,12 +70,17 @@
         def handler(signum, frame):
             if not posixpath.exists(self.filename):
                 self._manager.reactor.stop_all()
-
+        
         signal.signal(signal.SIGIO, handler)
-        fd = os.open(directory,  os.O_RDONLY)
-
-        fcntl.fcntl(fd, fcntl.F_SETSIG, 0)
-        fcntl.fcntl(fd, fcntl.F_NOTIFY, fcntl.DN_DELETE|fcntl.DN_MULTISHOT)
-
+        self._fd = os.open(directory,  os.O_RDONLY)
+
+        fcntl.fcntl(self._fd, fcntl.F_SETSIG, 0)
+        fcntl.fcntl(self._fd, fcntl.F_NOTIFY, fcntl.DN_DELETE|fcntl.DN_MULTISHOT)
+
+    def release(self):
+        # Properly release to the lock
+        self._lock.release(skip_delete=True)
+        os.close(self._fd)
+        os.unlink(self.filename)
 
 factory = LockPrompt
 
=== modified file 'plugins/persist_info.py'
--- plugins/persist_info.py	2012-06-20 20:33:09 +0000
+++ plugins/persist_info.py	2013-04-19 21:23:27 +0000
@@ -40,7 +40,7 @@
             self._manager.reactor.call_on(rt, rh, -100)
 
         # Save persist data last
-        self._manager.reactor.call_on("stop", self.save, 1000)
+        self._manager.reactor.call_on("stop", self.save, 900)
 
         #This should fire first thing during the gathering phase.
         self._manager.reactor.call_on("gather", self.begin_gather, -900)