[PATCH 1/7] event: Don't write duplicate logs to stdout and stderr in no UI case

[PATCH 1/7] event: Don't write duplicate logs to stdout and stderr in no UI case

[Richard Purdie]

This code would duplicate messages to stdout and stderr when no UI connected
and there were error level messages.

Rework the code so it either uses stderr (for errors and above) or
stdout for warnings/debug but not both for the same messages.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/event.py | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/lib/bb/event.py b/lib/bb/event.py
index 3827dcf..526c41f 100644
--- a/lib/bb/event.py
+++ b/lib/bb/event.py
@@ -149,23 +149,30 @@ def print_ui_queue():
 
         # First check to see if we have any proper messages
         msgprint = False
+        msgerrs = False
+
+        # Should we print to stderr?
+        for event in ui_queue[:]:
+            if isinstance(event, logging.LogRecord) and event.levelno >= logging.WARNING:
+                msgerrs = True
+                break
+
+        if msgerrs:
+            logger.addHandler(stderr)
+        else:
+            logger.addHandler(stdout)
+
         for event in ui_queue[:]:
             if isinstance(event, logging.LogRecord):
                 if event.levelno > logging.DEBUG:
-                    if event.levelno >= logging.WARNING:
-                        logger.addHandler(stderr)
-                    else:
-                        logger.addHandler(stdout)
                     logger.handle(event)
                     msgprint = True
-        if msgprint:
-            return
 
         # Nope, so just print all of the messages we have (including debug messages)
-        logger.addHandler(stdout)
-        for event in ui_queue[:]:
-            if isinstance(event, logging.LogRecord):
-                logger.handle(event)
+        if not msgprint:
+            for event in ui_queue[:]:
+                if isinstance(event, logging.LogRecord):
+                    logger.handle(event)
 
 def fire_ui_handlers(event, d):
     global _thread_lock
-- 
2.7.4

[PATCH 2/7] process: Ensure ConnectionReader/Writer have fileno() and close() methods

[Richard Purdie]

Expose the underlying close() and fileno() methods which allow connection
monitoring and cleanup.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/server/process.py | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/bb/server/process.py b/lib/bb/server/process.py
index 1975bd4..8a7c431 100644
--- a/lib/bb/server/process.py
+++ b/lib/bb/server/process.py
@@ -523,6 +523,10 @@ class ConnectionReader(object):
     def fileno(self):
         return self.reader.fileno()
 
+    def close(self):
+        return self.reader.close()
+
+
 class ConnectionWriter(object):
 
     def __init__(self, fd):
@@ -536,3 +540,8 @@ class ConnectionWriter(object):
         with self.wlock:
             self.writer.send_bytes(obj)
 
+    def fileno(self):
+        return self.writer.fileno()
+
+    def close(self):
+        return self.writer.close()
-- 
2.7.4

[PATCH 3/7] process: Allow BBUIEventQueue to exit cleanly

[Richard Purdie]

Currently the monitoring thread exits with some error code or runs indefinitely. Allow
closure of the pipe its monitoring to have the thread exit cleanly/silently.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/server/process.py | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/lib/bb/server/process.py b/lib/bb/server/process.py
index 8a7c431..3d9077f 100644
--- a/lib/bb/server/process.py
+++ b/lib/bb/server/process.py
@@ -499,9 +499,14 @@ class BBUIEventQueue:
     def startCallbackHandler(self):
         bb.utils.set_process_name("UIEventQueue")
         while True:
-            self.reader.wait()
-            event = self.reader.get()
-            self.queue_event(event)
+            try:
+                self.reader.wait()
+                event = self.reader.get()
+                self.queue_event(event)
+            except EOFError:
+                # Easiest way to exit is to close the file descriptor to cause an exit
+                break
+        self.reader.close()
 
 class ConnectionReader(object):
 
-- 
2.7.4

[PATCH 4/7] process: Move socket keep alive into BitBakeProcessServerConnection

[Richard Purdie]

This cleans up the socket keep alive into better class structured code
and adds cleanup of the open file descriptors upon shutdown.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/server/process.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/bb/server/process.py b/lib/bb/server/process.py
index 3d9077f..fb96804 100644
--- a/lib/bb/server/process.py
+++ b/lib/bb/server/process.py
@@ -336,12 +336,16 @@ class ServerCommunicator():
         return
 
 class BitBakeProcessServerConnection(object):
-    def __init__(self, ui_channel, recv, eq):
+    def __init__(self, ui_channel, recv, eq, sock):
         self.connection = ServerCommunicator(ui_channel, recv)
         self.events = eq
+        # Save sock so it doesn't get gc'd for the life of our connection
+        self.socket_connection = sock
 
     def terminate(self):
         self.socket_connection.close()
+        self.connection.connection.close()
+        self.connection.recv.close()
         return
 
 class BitBakeServer(object):
@@ -413,12 +417,10 @@ def connectProcessServer(sockname, featureset):
 
         sendfds(sock, [writefd, readfd1, writefd2])
 
-        server_connection = BitBakeProcessServerConnection(command_chan, command_chan_recv, eq)
+        server_connection = BitBakeProcessServerConnection(command_chan, command_chan_recv, eq, sock)
 
         server_connection.connection.updateFeatureSet(featureset)
 
-        # Save sock so it doesn't get gc'd for the life of our connection
-        server_connection.socket_connection = sock
     except:
         sock.close()
         raise
-- 
2.7.4

[PATCH 5/7] process/cooker: Allow UI process to know if the cooker was started successfully

[Richard Purdie]

Currently if the server fails to start, the user sees no error message and
the server will be repeatedly attempted to be started until some longer
timeouts expire. There are error messages in the cookerdeamon log but
nobody thinks to look there.

Add in a pipe which can be used to tell the starting process whether the cooker
did actually start or not. If it fails to start, no further attempts can be
made and if present, the log file can be shown to the user.

[YOCTO #11834]

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/cooker.py         |  6 +++++-
 lib/bb/server/process.py | 20 ++++++++++++++++++--
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index d6e6919..1a5e003 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -165,7 +165,7 @@ class BBCooker:
     Manages one bitbake build run
     """
 
-    def __init__(self, configuration, featureSet=None):
+    def __init__(self, configuration, featureSet=None, readypipe=None):
         self.recipecaches = None
         self.skiplist = {}
         self.featureset = CookerFeatures()
@@ -237,6 +237,10 @@ class BBCooker:
         # Let SIGHUP exit as SIGTERM
         signal.signal(signal.SIGHUP, self.sigterm_exception)
 
+        if readypipe:
+            os.write(readypipe, b"ready")
+            os.close(readypipe)
+
     def config_notifications(self, event):
         if event.maskname == "IN_Q_OVERFLOW":
             bb.warn("inotify event queue overflowed, invalidating caches.")
diff --git a/lib/bb/server/process.py b/lib/bb/server/process.py
index fb96804..b9bde46 100644
--- a/lib/bb/server/process.py
+++ b/lib/bb/server/process.py
@@ -355,6 +355,7 @@ class BitBakeServer(object):
         self.featureset = featureset
         self.sockname = sockname
         self.bitbake_lock = lock
+        self.readypipe, self.readypipein = os.pipe()
 
         # Create server control socket
         if os.path.exists(sockname):
@@ -363,6 +364,8 @@ class BitBakeServer(object):
         self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
         # AF_UNIX has path length issues so chdir here to workaround
         cwd = os.getcwd()
+        logfile = os.path.join(cwd, "bitbake-cookerdaemon.log")
+
         try:
             os.chdir(os.path.dirname(sockname))
             self.sock.bind(os.path.basename(sockname))
@@ -371,10 +374,23 @@ class BitBakeServer(object):
         self.sock.listen(1)
 
         os.set_inheritable(self.sock.fileno(), True)
-        bb.daemonize.createDaemon(self._startServer, "bitbake-cookerdaemon.log")
+        bb.daemonize.createDaemon(self._startServer, logfile)
         self.sock.close()
         self.bitbake_lock.close()
 
+        ready = ConnectionReader(self.readypipe)
+        r = ready.wait(3)
+        if not r:
+            ready.close()
+            bb.error("Unable to start bitbake server")
+            if os.path.exists(logfile):
+                with open(logfile, "r") as f:
+                    logs=f.readlines()
+                bb.error("Last 10 lines of server log %s:\n%s" % (logfile, "".join(logs[-10:])))
+            raise SystemExit(1)
+        ready.close()
+        os.close(self.readypipein)
+
     def _startServer(self):
         server = ProcessServer(self.bitbake_lock, self.sock, self.sockname)
         self.configuration.setServerRegIdleCallback(server.register_idle_function)
@@ -385,7 +401,7 @@ class BitBakeServer(object):
             if value:
                 setattr(self.configuration, "%s_server" % param, value)
 
-        self.cooker = bb.cooker.BBCooker(self.configuration, self.featureset)
+        self.cooker = bb.cooker.BBCooker(self.configuration, self.featureset, self.readypipein)
         server.cooker = self.cooker
         server.server_timeout = self.configuration.server_timeout
         server.xmlrpcinterface = self.configuration.xmlrpcinterface
-- 
2.7.4

[PATCH 6/7] process: Don't leak open pipes upon reconnection

[Richard Purdie]

If we reconnect to the server, stop leaking pipes and clean up
after ourselves.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/server/process.py | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/lib/bb/server/process.py b/lib/bb/server/process.py
index b9bde46..e2db480 100644
--- a/lib/bb/server/process.py
+++ b/lib/bb/server/process.py
@@ -420,7 +420,11 @@ def connectProcessServer(sockname, featureset):
     finally:
         os.chdir(cwd)
 
+    readfd = writefd = readfd1 = writefd1 = readfd2 = writefd2 = None
+    eq = command_chan_recv = command_chan = None
+
     try:
+
         # Send an fd for the remote to write events to
         readfd, writefd = os.pipe()
         eq = BBUIEventQueue(readfd)
@@ -435,9 +439,22 @@ def connectProcessServer(sockname, featureset):
 
         server_connection = BitBakeProcessServerConnection(command_chan, command_chan_recv, eq, sock)
 
+        # Close the ends of the pipes we won't use
+        for i in [writefd, readfd1, writefd2]:
+            os.close(i)
+
         server_connection.connection.updateFeatureSet(featureset)
 
-    except:
+    except (Exception, SystemExit) as e:
+        if command_chan_recv:
+            command_chan_recv.close()
+        if command_chan:
+            command_chan.close()
+        for i in [writefd, readfd1, writefd2]:
+            try:
+                os.close(i)
+            except OSError:
+                pass
         sock.close()
         raise
 
-- 
2.7.4

[PATCH 7/7] process: Clean up server communication timeout errors

[Richard Purdie]

This timeout path was commonly hit due to errors starting the server. Now we
have a better way to handle that, the retry logic can be improved and cleaned
up. This patch:

* Makes the timeout 5s rather than intervals of 1s with a message. Paul
  noted some commands can take around 1s to run on a server which has just
  been started on a loaded system.
* Allows a broke connection to exit immediately rather than retrying something
  which will never work.
* Drops the Ctrl+C masking, we shouldn't need that anymore and any issues
  would be better handled in other ways.

This should make things clearer and less confusing for users and is much cleaner
code too.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/server/process.py | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/lib/bb/server/process.py b/lib/bb/server/process.py
index e2db480..60ef190 100644
--- a/lib/bb/server/process.py
+++ b/lib/bb/server/process.py
@@ -303,19 +303,10 @@ class ServerCommunicator():
         self.recv = recv
 
     def runCommand(self, command):
-
         self.connection.send(command)
-        while True:
-            # don't let the user ctrl-c while we're waiting for a response
-            try:
-                for idx in range(0,4): # 0, 1, 2, 3
-                    if self.recv.poll(1):
-                        return self.recv.get()
-                    else:
-                        bb.note("Timeout while attempting to communicate with bitbake server, retrying...")
-                raise ProcessTimeout("Gave up; Too many tries: timeout while attempting to communicate with bitbake server")
-            except KeyboardInterrupt:
-                pass
+        if not self.recv.poll(5):
+            raise ProcessTimeout("Timeout while waiting for a reply from the bitbake server")
+        return self.recv.get()
 
     def updateFeatureSet(self, featureset):
         _, error = self.runCommand(["setFeatures", featureset])
-- 
2.7.4