Skip to contentSkip to conversation

Fix for bitbake.lock handling

PATCH New Paul Eggleton 1 participant 2 messages Jul 31, 2015

lore
Share & tools
lore
#0 lore raw

cover Fix for bitbake.lock handling

A patch from Richard and myself to try to fix bitbake.lock handling properly this time.

The following changes since commit bef724057f1ea81571dd3ac5a9cf862f818434b5:

build: Exit scripts with result of last shell function (2015-07-27 16:29:11 +0100)

are available in the git repository at:

git://git.yoctoproject.org/poky-contrib paule/bitbake-lock-fix
http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=paule/bitbake-lock-fix

Richard Purdie (1):
cooker: properly fix bitbake.lock handling

 lib/bb/command.py   |  6 ------
 lib/bb/cooker.py    | 30 ++++++++++++++++++++++++++++++
 lib/bb/main.py      |  1 +
 lib/bb/tinfoil.py   |  1 +
 lib/bb/ui/knotty.py | 20 --------------------
 lib/bb/utils.py     | 29 +++++++++++++++++++++++++----
 6 files changed, 57 insertions(+), 30 deletions(-)
Signature · 1 line
2.1.0
#1 lore raw

PATCH 1/1 cooker: properly fix bitbake.lock handling

From: Richard Purdie <[email protected]>

If the PR server or indeed any other child process takes some time to exit (which it sometimes does when saving its database), it can end up holding bitbake.lock after the UI exits, which led to errors if you ran bitbake commands successively - we saw this when running the PR server oe-selftest tests in OE-Core. The recent attempt to fix this wasn't quite right and ended up breaking memory resident bitbake. This time we close the lock file when cooker shuts down (inside the UI process) instead of unlocking it, and this is done in the cooker code rather than the actual UI code so it doesn't matter which UI is in use. Additionally we report that we're waiting for the lock to be released, using lsof or fuser if available to list the processes with the lock open.

(The reproducer for the problem is to set PRSERV_HOST = "localhost:0" and add a call to time.sleep(20) after self.server_close() in lib/prserv/serv.py, then run bitbake -p ).

Cleanup work done by Paul Eggleton <[email protected]>.

This reverts bitbake commit 69ecd15aece54753154950c55d7af42f85ad8606 and e97a9f1528d77503b5c93e48e3de9933fbb9f3cd.

---
 lib/bb/command.py   |  6 ------
 lib/bb/cooker.py    | 30 ++++++++++++++++++++++++++++++
 lib/bb/main.py      |  1 +
 lib/bb/tinfoil.py   |  1 +
 lib/bb/ui/knotty.py | 20 --------------------
 lib/bb/utils.py     | 29 +++++++++++++++++++++++++----
 6 files changed, 57 insertions(+), 30 deletions(-)
lib/bb/command.py +0 -6
index 2b02584..24ff341 100644
@@ -267,12 +267,6 @@ class CommandsSync:
         features = params[0]
         command.cooker.setFeatures(features)
 
-    def unlockBitbake(self, command, params):
-        """
-        Unlock bitbake.lock file
-        """
-        command.cooker.unlockBitbake()
-
     # although we change the internal state of the cooker, this is transparent since
     # we always take and leave the cooker in state.initial
     setFeatures.readonly = True
lib/bb/cooker.py +30 -0
index 46e5560..52b81be 100644
@@ -38,6 +38,8 @@ import bb, bb.exceptions, bb.command
 from bb import utils, data, parse, event, cache, providers, taskdata, runqueue, build
 import Queue
 import signal
+import subprocess
+import errno
 import prserv.serv
 import pyinotify
 
@@ -1532,6 +1534,34 @@ class BBCooker:
     def post_serve(self):
         prserv.serv.auto_shutdown(self.data)
         bb.event.fire(CookerExit(), self.expanded_data)
+        lockfile = self.lock.name
+        self.lock.close()
+        self.lock = None
+
+        while not self.lock:
+            with bb.utils.timeout(3):
+                self.lock = bb.utils.lockfile(lockfile, shared=False, retry=False, block=True)
+                if not self.lock:
+                    # Some systems may not have lsof available
+                    procs = None
+                    try:
+                        procs = subprocess.check_output(["lsof", '-w', lockfile], stderr=subprocess.STDOUT)
+                    except OSError as e:
+                        if e.errno != errno.ENOENT:
+                            raise
+                    if procs is None:
+                        # Fall back to fuser if lsof is unavailable
+                        try:
+                            procs = subprocess.check_output(["fuser", '-v', lockfile], stderr=subprocess.STDOUT)
+                        except OSError as e:
+                            if e.errno != errno.ENOENT:
+                                raise
+
+                    msg = "Delaying shutdown due to active processes which appear to be holding bitbake.lock"
+                    if procs:
+                        msg += ":\n%s" % str(procs)
+                    print(msg)
+
 
     def shutdown(self, force = False):
         if force:
lib/bb/main.py +1 -0
index ee72328..f08a8ef 100755
@@ -257,6 +257,7 @@ def start_server(servermodule, configParams, configuration, features):
                 logger.handle(event)
         raise exc_info[1], None, exc_info[2]
     server.detach()
+    cooker.lock.close()
     return server
 
 
lib/bb/tinfoil.py +1 -0
index 3095c20..1ea46d8 100644
@@ -86,6 +86,7 @@ class Tinfoil:
 
     def shutdown(self):
         self.cooker.shutdown(force=True)
+        self.cooker.post_serve()
         self.cooker.unlockBitbake()
 
 class TinfoilConfigParameters(ConfigParameters):
lib/bb/ui/knotty.py +0 -20
index 9788a92..2bee242 100644
@@ -310,7 +310,6 @@ def main(server, eventHandler, params, tf = TerminalFilter):
     errors = 0
     warnings = 0
     taskfailures = []
-    locktries = 10
 
     termfilter = tf(main, helper, console, errconsole, format)
     atexit.register(termfilter.finish)
@@ -538,25 +537,6 @@ def main(server, eventHandler, params, tf = TerminalFilter):
                 _, error = server.runCommand(["stateForceShutdown"])
             main.shutdown = 2
     try:
-        topdir, error = server.runCommand(["getVariable", "TOPDIR"])
-        if error:
-            logger.warn("Unable to get the value of TOPDIR variable: %s" % error)
-        else:
-            lockfile = "%s/bitbake.lock" % topdir
-            _, error = server.runCommand(["unlockBitbake"])
-            if error:
-                logger.warn("Unable to unlock the file %s" % lockfile)
-            else:
-                while locktries:
-                    lf = bb.utils.lockfile(lockfile, False, False)
-                    if not lf:
-                        time.sleep(1)
-                        locktries -=1
-                    else:
-                        bb.utils.unlockfile(lf)
-                        break
-                if not locktries:
-                    logger.warn("Knotty could not lock the file ${TOPDIR}/bitbake.lock, probably locked by cooker and not unlocked yet. Immediate bitbake commands may failed")
         summary = ""
         if taskfailures:
             summary += pluralise("\nSummary: %s task failed:",
lib/bb/utils.py +25 -4
index 857f5bc..607ffc5 100644
@@ -31,6 +31,7 @@ import subprocess
 import glob
 import traceback
 import errno
+import signal
 from commands import getstatusoutput
 from contextlib import contextmanager
 
@@ -412,10 +413,30 @@ def fileslocked(files):
     for lock in locks:
         bb.utils.unlockfile(lock)
 
-def lockfile(name, shared=False, retry=True):
+@contextmanager
+def timeout(seconds):
+    def timeout_handler(signum, frame):
+        pass
+
+    original_handler = signal.signal(signal.SIGALRM, timeout_handler)
+
+    try:
+        signal.alarm(seconds)
+        yield
+    finally:
+        signal.alarm(0)
+        signal.signal(signal.SIGALRM, original_handler)
+
+def lockfile(name, shared=False, retry=True, block=False):
     """
-    Use the file fn as a lock file, return when the lock has been acquired.
-    Returns a variable to pass to unlockfile().
+    Use the specified file as a lock file, return when the lock has
+    been acquired. Returns a variable to pass to unlockfile().
+    Parameters:
+        retry: True to re-try locking if it fails, False otherwise
+        block: True to block until the lock succeeds, False otherwise
+    The retry and block parameters are kind of equivalent unless you
+    consider the possibility of sending a signal to the process to break
+    out - at which point you want block=True rather than retry=True.
     """
     dirname = os.path.dirname(name)
     mkdirhier(dirname)
@@ -428,7 +449,7 @@ def lockfile(name, shared=False, retry=True):
     op = fcntl.LOCK_EX
     if shared:
         op = fcntl.LOCK_SH
-    if not retry:
+    if not retry and not block:
         op = op | fcntl.LOCK_NB
 
     while True:
Signature · 1 line
2.1.0