All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/15] Bitbake server thread enabling
@ 2022-12-29 17:07 Richard Purdie
  2022-12-29 17:07 ` [PATCH 01/15] command: Tweak finishAsyncCommand ordering to avoid races Richard Purdie
                   ` (14 more replies)
  0 siblings, 15 replies; 18+ messages in thread
From: Richard Purdie @ 2022-12-29 17:07 UTC (permalink / raw)
  To: bitbake-devel

This series adds 'idle' thread support to bitbake. These changes have
struggled a bit in testing as we're trying to add threading support to
code which never had it originally. We've been aiming at this for
some time (over a decade) but have reached the point where we need to 
do it and deal with any issues arrising.

The 'idle' thread is badly named as it is where the "real work"
of bitbake gets done, e.g. runqueue (task execution) is run as an 
idle handler. We can worry about renaming things later, getting the
basic changes working is my concern right now.

The problem these changes solve:
  * allows the UI to tell if the server is alive and still there
    even when async commands are running
  * allows the UI to interrupt the server and tell it to shutdown
    (i.e. make Ctrl+C repsonsive from the UI)
  * allow us to make progress on the various "bitbake hanging" bugs
    we have where the UI can't connect

I believe the changes are at the point where they should be able to 
merge. The series is a bit messy as the changes build on each other
and there are various fixes for various pieces of code along the way.
I could try and separate them into a different order but they're 
intertwined and I think the current form shows the issues each change
addresses which would be lost if it were restructured.

There is much more work needed here and the code can be considerably
cleaned up to make it clearer and most accessible, those changes
can follow and build from this though.

Richard Purdie (15):
  command: Tweak finishAsyncCommand ordering to avoid races
  cooker: Ensure commands clean up any parser processes
  knotty: Ping the server/cooker periodically
  event: Add enable/disable heartbeat code
  server/process: Run idle commands in a separate idle thread
  server/process: Improve idle loop exit code
  process: Improve client disconnect/idle sync
  process: Improve async command execution with idle interaction
  knotty: Avoid looping with tracebacks
  event: Always use threadlock
  server/process: Improve exception logging
  cooker/cookerdata: Rework the way the datastores are reset
  cooker: Ensure we clean up active idle handlers
  cache: Drop reciever side counting for SiggenRecipeInfo
  server/process: Add debug to show which handlers are active

 lib/bb/cache.py               |  15 ++---
 lib/bb/command.py             |  27 ++++-----
 lib/bb/cooker.py              |  58 ++++++++++++--------
 lib/bb/cookerdata.py          |  31 +++++++----
 lib/bb/event.py               |  82 +++++++++++++++-------------
 lib/bb/server/process.py      | 100 ++++++++++++++++++++++++----------
 lib/bb/server/xmlrpcserver.py |   2 +-
 lib/bb/tests/event.py         |  17 +-----
 lib/bb/ui/knotty.py           |  16 ++++--
 9 files changed, 206 insertions(+), 142 deletions(-)

-- 
2.37.2



^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 01/15] command: Tweak finishAsyncCommand ordering to avoid races
  2022-12-29 17:07 [PATCH 00/15] Bitbake server thread enabling Richard Purdie
@ 2022-12-29 17:07 ` Richard Purdie
  2022-12-29 17:07 ` [PATCH 02/15] cooker: Ensure commands clean up any parser processes Richard Purdie
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Richard Purdie @ 2022-12-29 17:07 UTC (permalink / raw)
  To: bitbake-devel

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/command.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bb/command.py b/lib/bb/command.py
index 0208e30ec4..cbac07f516 100644
--- a/lib/bb/command.py
+++ b/lib/bb/command.py
@@ -152,8 +152,8 @@ class Command:
             bb.event.fire(CommandExit(code), self.cooker.data)
         else:
             bb.event.fire(CommandCompleted(), self.cooker.data)
-        self.currentAsyncCommand = None
         self.cooker.finishcommand()
+        self.currentAsyncCommand = None
 
     def reset(self):
         if self.remotedatastores:
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 02/15] cooker: Ensure commands clean up any parser processes
  2022-12-29 17:07 [PATCH 00/15] Bitbake server thread enabling Richard Purdie
  2022-12-29 17:07 ` [PATCH 01/15] command: Tweak finishAsyncCommand ordering to avoid races Richard Purdie
@ 2022-12-29 17:07 ` Richard Purdie
  2022-12-29 17:07 ` [PATCH 03/15] knotty: Ping the server/cooker periodically Richard Purdie
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Richard Purdie @ 2022-12-29 17:07 UTC (permalink / raw)
  To: bitbake-devel

When finishing a command, we need to ensure any parsing processes that may have
been started are cleaned up before we reset the cooker state.

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

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index 4e49b0e90b..df31a413d7 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -1768,6 +1768,9 @@ class BBCooker:
             self.parser.final_cleanup()
 
     def finishcommand(self):
+        if hasattr(self.parser, 'shutdown'):
+            self.parser.shutdown(clean=False)
+            self.parser.final_cleanup()
         self.state = state.initial
 
     def reset(self):
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 03/15] knotty: Ping the server/cooker periodically
  2022-12-29 17:07 [PATCH 00/15] Bitbake server thread enabling Richard Purdie
  2022-12-29 17:07 ` [PATCH 01/15] command: Tweak finishAsyncCommand ordering to avoid races Richard Purdie
  2022-12-29 17:07 ` [PATCH 02/15] cooker: Ensure commands clean up any parser processes Richard Purdie
@ 2022-12-29 17:07 ` Richard Purdie
  2022-12-30 15:44   ` [bitbake-devel] " Peter Kjellerstedt
  2022-12-29 17:07 ` [PATCH 04/15] event: Add enable/disable heartbeat code Richard Purdie
                   ` (11 subsequent siblings)
  14 siblings, 1 reply; 18+ messages in thread
From: Richard Purdie @ 2022-12-29 17:07 UTC (permalink / raw)
  To: bitbake-devel

We've seeing failures where the UI hangs if the server disappears. Ping
the cooker/server if we've not had any events in the last minute so we can
check if it is still alive.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/ui/knotty.py | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/bb/ui/knotty.py b/lib/bb/ui/knotty.py
index 61cf0a37f4..ab1a367be0 100644
--- a/lib/bb/ui/knotty.py
+++ b/lib/bb/ui/knotty.py
@@ -625,7 +625,8 @@ def main(server, eventHandler, params, tf = TerminalFilter):
 
     printintervaldelta = 10 * 60 # 10 minutes
     printinterval = printintervaldelta
-    lastprint = time.time()
+    pinginterval = 1 * 60 # 1 minutes
+    lastevent = lastprint = time.time()
 
     termfilter = tf(main, helper, console_handlers, params.options.quiet)
     atexit.register(termfilter.finish)
@@ -637,6 +638,14 @@ def main(server, eventHandler, params, tf = TerminalFilter):
                 printinterval += printintervaldelta
             event = eventHandler.waitEvent(0)
             if event is None:
+                if (lastevent + pinginterval) <= time.time():
+                    ret, error = server.runCommand(["ping"])
+                    if error or not ret:
+                        termfilter.clearFooter()
+                        print("No reply after pinging server (%s, %s), exiting." % (str(error), str(ret)))
+                        return_value = 3
+                        main.shutdown = 2
+                    lastevent = time.time()
                 if main.shutdown > 1:
                     break
                 if not parseprogress:
@@ -644,6 +653,7 @@ def main(server, eventHandler, params, tf = TerminalFilter):
                 event = eventHandler.waitEvent(0.25)
                 if event is None:
                     continue
+            lastevent = time.time()
             helper.eventHandler(event)
             if isinstance(event, bb.runqueue.runQueueExitWait):
                 if not main.shutdown:
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 04/15] event: Add enable/disable heartbeat code
  2022-12-29 17:07 [PATCH 00/15] Bitbake server thread enabling Richard Purdie
                   ` (2 preceding siblings ...)
  2022-12-29 17:07 ` [PATCH 03/15] knotty: Ping the server/cooker periodically Richard Purdie
@ 2022-12-29 17:07 ` Richard Purdie
  2022-12-29 17:07 ` [PATCH 05/15] server/process: Run idle commands in a separate idle thread Richard Purdie
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Richard Purdie @ 2022-12-29 17:07 UTC (permalink / raw)
  To: bitbake-devel

Currently heartbeat events are always generated by the server whilst it is
active. Change this so they only appear when builds are running, which is
when most code would expect to be executed. This removes a number of races
around changes in the datastore which can happen outside of builds.

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

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index df31a413d7..527be2f649 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -1467,6 +1467,7 @@ class BBCooker:
         buildname = self.databuilder.mcdata[mc].getVar("BUILDNAME")
         if fireevents:
             bb.event.fire(bb.event.BuildStarted(buildname, [item]), self.databuilder.mcdata[mc])
+            bb.event.enable_heartbeat()
 
         # Execute the runqueue
         runlist = [[mc, item, task, fn]]
@@ -1500,6 +1501,7 @@ class BBCooker:
             if not retval:
                 if fireevents:
                     bb.event.fire(bb.event.BuildCompleted(len(rq.rqdata.runtaskentries), buildname, item, failures, interrupted), self.databuilder.mcdata[mc])
+                    bb.event.disable_heartbeat()
                 self.command.finishAsyncCommand(msg)
                 # We trashed self.recipecaches above
                 self.parsecache_valid = False
@@ -1545,6 +1547,7 @@ class BBCooker:
                     for mc in self.multiconfigs:
                         bb.event.fire(bb.event.BuildCompleted(len(rq.rqdata.runtaskentries), buildname, targets, failures, interrupted), self.databuilder.mcdata[mc])
                 finally:
+                    bb.event.disable_heartbeat()
                     self.command.finishAsyncCommand(msg)
                 return False
             if retval is True:
@@ -1578,6 +1581,7 @@ class BBCooker:
 
         for mc in self.multiconfigs:
             bb.event.fire(bb.event.BuildStarted(buildname, ntargets), self.databuilder.mcdata[mc])
+        bb.event.enable_heartbeat()
 
         rq = bb.runqueue.RunQueue(self, self.data, self.recipecaches, taskdata, runlist)
         if 'universe' in targets:
diff --git a/lib/bb/event.py b/lib/bb/event.py
index 303b7a943f..db90724444 100644
--- a/lib/bb/event.py
+++ b/lib/bb/event.py
@@ -69,6 +69,7 @@ _eventfilter = None
 _uiready = False
 _thread_lock = threading.Lock()
 _thread_lock_enabled = False
+_heartbeat_enabled = False
 
 def enable_threadlock():
     global _thread_lock_enabled
@@ -78,6 +79,14 @@ def disable_threadlock():
     global _thread_lock_enabled
     _thread_lock_enabled = False
 
+def enable_heartbeat():
+    global _heartbeat_enabled
+    _heartbeat_enabled = True
+
+def disable_heartbeat():
+    global _heartbeat_enabled
+    _heartbeat_enabled = False
+
 def execute_handler(name, handler, event, d):
     event.data = d
     try:
diff --git a/lib/bb/server/process.py b/lib/bb/server/process.py
index 586d46af88..91eb6e0ad9 100644
--- a/lib/bb/server/process.py
+++ b/lib/bb/server/process.py
@@ -382,7 +382,7 @@ class ProcessServer():
 
         # Create new heartbeat event?
         now = time.time()
-        if now >= self.next_heartbeat:
+        if bb.event._heartbeat_enabled and now >= self.next_heartbeat:
             # We might have missed heartbeats. Just trigger once in
             # that case and continue after the usual delay.
             self.next_heartbeat += self.heartbeat_seconds
@@ -396,7 +396,7 @@ class ProcessServer():
                     if not isinstance(exc, bb.BBHandledException):
                         logger.exception('Running heartbeat function')
                     self.quit = True
-        if nextsleep and now + nextsleep > self.next_heartbeat:
+        if nextsleep and bb.event._heartbeat_enabled and now + nextsleep > self.next_heartbeat:
             # Shorten timeout so that we we wake up in time for
             # the heartbeat.
             nextsleep = self.next_heartbeat - now
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 05/15] server/process: Run idle commands in a separate idle thread
  2022-12-29 17:07 [PATCH 00/15] Bitbake server thread enabling Richard Purdie
                   ` (3 preceding siblings ...)
  2022-12-29 17:07 ` [PATCH 04/15] event: Add enable/disable heartbeat code Richard Purdie
@ 2022-12-29 17:07 ` Richard Purdie
  2022-12-29 17:07 ` [PATCH 06/15] server/process: Improve idle loop exit code Richard Purdie
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Richard Purdie @ 2022-12-29 17:07 UTC (permalink / raw)
  To: bitbake-devel

When bitbake is off running heavier "idle" commands, it doesn't service it's
command socket which means stopping/interrupting it is hard. It also means we
can't "ping" from the UI to know if it is still alive.

For those reasons, split idle command execution into it's own thread.

The commands are generally already self containted so this is easier than
expected. We do have to be careful to only handle inotify poll() from a single
thread at a time. It also means we always have to use a thread lock when sending
events since both the idle thread and the command thread may generate log messages
(and hence events). The patch does depend on a couple of previous fixes to the
builtins locking in event.py and the heartbeat enable/disable changes.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/command.py        |  4 +--
 lib/bb/cooker.py         | 19 +++++++++----
 lib/bb/server/process.py | 60 +++++++++++++++++++++++++---------------
 3 files changed, 54 insertions(+), 29 deletions(-)

diff --git a/lib/bb/command.py b/lib/bb/command.py
index cbac07f516..d2c01bf743 100644
--- a/lib/bb/command.py
+++ b/lib/bb/command.py
@@ -84,7 +84,7 @@ class Command:
                 if not hasattr(command_method, 'readonly') or not getattr(command_method, 'readonly'):
                     return None, "Not able to execute not readonly commands in readonly mode"
             try:
-                self.cooker.process_inotify_updates()
+                self.cooker.process_inotify_updates_apply()
                 if getattr(command_method, 'needconfig', True):
                     self.cooker.updateCacheSync()
                 result = command_method(self, commandline)
@@ -109,7 +109,7 @@ class Command:
 
     def runAsyncCommand(self):
         try:
-            self.cooker.process_inotify_updates()
+            self.cooker.process_inotify_updates_apply()
             if self.cooker.state in (bb.cooker.state.error, bb.cooker.state.shutdown, bb.cooker.state.forceshutdown):
                 # updateCache will trigger a shutdown of the parser
                 # and then raise BBHandledException triggering an exit
diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index 527be2f649..33e87981c5 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -220,6 +220,8 @@ class BBCooker:
         bb.debug(1, "BBCooker startup complete %s" % time.time())
         sys.stdout.flush()
 
+        self.inotify_threadlock = threading.Lock()
+
     def init_configdata(self):
         if not hasattr(self, "data"):
             self.initConfigurationData()
@@ -248,11 +250,18 @@ class BBCooker:
         self.notifier = pyinotify.Notifier(self.watcher, self.notifications)
 
     def process_inotify_updates(self):
-        for n in [self.confignotifier, self.notifier]:
-            if n and n.check_events(timeout=0):
-                # read notified events and enqueue them
-                n.read_events()
-                n.process_events()
+        with self.inotify_threadlock:
+            for n in [self.confignotifier, self.notifier]:
+                if n and n.check_events(timeout=0):
+                    # read notified events and enqueue them
+                    n.read_events()
+
+    def process_inotify_updates_apply(self):
+        with self.inotify_threadlock:
+            for n in [self.confignotifier, self.notifier]:
+                if n and n.check_events(timeout=0):
+                    n.read_events()
+                    n.process_events()
 
     def config_notifications(self, event):
         if event.maskname == "IN_Q_OVERFLOW":
diff --git a/lib/bb/server/process.py b/lib/bb/server/process.py
index 91eb6e0ad9..6f43330fae 100644
--- a/lib/bb/server/process.py
+++ b/lib/bb/server/process.py
@@ -88,6 +88,7 @@ class ProcessServer():
         self.maxuiwait = 30
         self.xmlrpc = False
 
+        self.idle = None
         self._idlefuns = {}
 
         self.bitbake_lock = lock
@@ -148,6 +149,7 @@ class ProcessServer():
         self.cooker.pre_serve()
 
         bb.utils.set_process_name("Cooker")
+        bb.event.enable_threadlock()
 
         ready = []
         newconnections = []
@@ -278,6 +280,9 @@ class ProcessServer():
 
             ready = self.idle_commands(.1, fds)
 
+        if self.idle:
+            self.idle.join()
+
         serverlog("Exiting (socket: %s)" % os.path.exists(self.sockname))
         # Remove the socket file so we don't get any more connections to avoid races
         # The build directory could have been renamed so if the file isn't the one we created
@@ -352,33 +357,44 @@ class ProcessServer():
                     msg.append(":\n%s" % procs)
                 serverlog("".join(msg))
 
+    def idle_thread(self):
+        while not self.quit:
+            nextsleep = 0.1
+            fds = []
+            for function, data in list(self._idlefuns.items()):
+                try:
+                    retval = function(self, data, False)
+                    if retval is False:
+                        del self._idlefuns[function]
+                        nextsleep = None
+                    elif retval is True:
+                        nextsleep = None
+                    elif isinstance(retval, float) and nextsleep:
+                        if (retval < nextsleep):
+                            nextsleep = retval
+                    elif nextsleep is None:
+                        continue
+                    else:
+                        fds = fds + retval
+                except SystemExit:
+                    raise
+                except Exception as exc:
+                    if not isinstance(exc, bb.BBHandledException):
+                        logger.exception('Running idle function')
+                    del self._idlefuns[function]
+                    self.quit = True
+
+            if nextsleep is not None:
+                select.select(fds,[],[],nextsleep)[0]
+
     def idle_commands(self, delay, fds=None):
         nextsleep = delay
         if not fds:
             fds = []
 
-        for function, data in list(self._idlefuns.items()):
-            try:
-                retval = function(self, data, False)
-                if retval is False:
-                    del self._idlefuns[function]
-                    nextsleep = None
-                elif retval is True:
-                    nextsleep = None
-                elif isinstance(retval, float) and nextsleep:
-                    if (retval < nextsleep):
-                        nextsleep = retval
-                elif nextsleep is None:
-                    continue
-                else:
-                    fds = fds + retval
-            except SystemExit:
-                raise
-            except Exception as exc:
-                if not isinstance(exc, bb.BBHandledException):
-                    logger.exception('Running idle function')
-                del self._idlefuns[function]
-                self.quit = True
+        if not self.idle:
+            self.idle = threading.Thread(target=self.idle_thread)
+            self.idle.start()
 
         # Create new heartbeat event?
         now = time.time()
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 06/15] server/process: Improve idle loop exit code
  2022-12-29 17:07 [PATCH 00/15] Bitbake server thread enabling Richard Purdie
                   ` (4 preceding siblings ...)
  2022-12-29 17:07 ` [PATCH 05/15] server/process: Run idle commands in a separate idle thread Richard Purdie
@ 2022-12-29 17:07 ` Richard Purdie
  2022-12-29 17:07 ` [PATCH 07/15] process: Improve client disconnect/idle sync Richard Purdie
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Richard Purdie @ 2022-12-29 17:07 UTC (permalink / raw)
  To: bitbake-devel

When idle handlers want to exit, returning "False" isn't very clear
and also causes challenges with the ordering of the removing the idle
handler and marking that no async command is running.

Use a specific class to signal the exit condition allowing clearer code
and allowing the async command to be cleared after the handler has been
removed, reducing any opportunity for races.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/command.py        | 13 +++++--------
 lib/bb/cooker.py         | 13 +++++--------
 lib/bb/server/process.py |  8 ++++++++
 3 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/lib/bb/command.py b/lib/bb/command.py
index d2c01bf743..d03f35a896 100644
--- a/lib/bb/command.py
+++ b/lib/bb/command.py
@@ -128,22 +128,19 @@ class Command:
             else:
                 return False
         except KeyboardInterrupt as exc:
-            self.finishAsyncCommand("Interrupted")
-            return False
+            return bb.server.process.idleFinish("Interrupted")
         except SystemExit as exc:
             arg = exc.args[0]
             if isinstance(arg, str):
-                self.finishAsyncCommand(arg)
+                return bb.server.process.idleFinish(arg)
             else:
-                self.finishAsyncCommand("Exited with %s" % arg)
-            return False
+                return bb.server.process.idleFinish("Exited with %s" % arg)
         except Exception as exc:
             import traceback
             if isinstance(exc, bb.BBHandledException):
-                self.finishAsyncCommand("")
+                return bb.server.process.idleFinish("")
             else:
-                self.finishAsyncCommand(traceback.format_exc())
-            return False
+                return bb.server.process.idleFinish(traceback.format_exc())
 
     def finishAsyncCommand(self, msg=None, code=None):
         if msg or msg == "":
diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index 33e87981c5..5e18c85bb8 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -1502,23 +1502,21 @@ class BBCooker:
                 failures += len(exc.args)
                 retval = False
             except SystemExit as exc:
-                self.command.finishAsyncCommand(str(exc))
                 if quietlog:
                     bb.runqueue.logger.setLevel(rqloglevel)
-                return False
+                return bb.server.process.idleFinish(str(exc))
 
             if not retval:
                 if fireevents:
                     bb.event.fire(bb.event.BuildCompleted(len(rq.rqdata.runtaskentries), buildname, item, failures, interrupted), self.databuilder.mcdata[mc])
                     bb.event.disable_heartbeat()
-                self.command.finishAsyncCommand(msg)
                 # We trashed self.recipecaches above
                 self.parsecache_valid = False
                 self.configuration.limited_deps = False
                 bb.parse.siggen.reset(self.data)
                 if quietlog:
                     bb.runqueue.logger.setLevel(rqloglevel)
-                return False
+                return bb.server.process.idleFinish(msg)
             if retval is True:
                 return True
             return retval
@@ -1548,8 +1546,7 @@ class BBCooker:
                 failures += len(exc.args)
                 retval = False
             except SystemExit as exc:
-                self.command.finishAsyncCommand(str(exc))
-                return False
+                return bb.server.process.idleFinish(str(exc))
 
             if not retval:
                 try:
@@ -1557,8 +1554,8 @@ class BBCooker:
                         bb.event.fire(bb.event.BuildCompleted(len(rq.rqdata.runtaskentries), buildname, targets, failures, interrupted), self.databuilder.mcdata[mc])
                 finally:
                     bb.event.disable_heartbeat()
-                    self.command.finishAsyncCommand(msg)
-                return False
+                return bb.server.process.idleFinish(msg)
+
             if retval is True:
                 return True
             return retval
diff --git a/lib/bb/server/process.py b/lib/bb/server/process.py
index 6f43330fae..3c909942aa 100644
--- a/lib/bb/server/process.py
+++ b/lib/bb/server/process.py
@@ -71,6 +71,10 @@ def get_lockfile_process_msg(lockfile):
         return procs.decode("utf-8")
     return None
 
+class idleFinish():
+    def __init__(self, msg):
+         self.msg = msg
+
 class ProcessServer():
     profile_filename = "profile.log"
     profile_processed_filename = "profile.log.processed"
@@ -364,6 +368,10 @@ class ProcessServer():
             for function, data in list(self._idlefuns.items()):
                 try:
                     retval = function(self, data, False)
+                    if isinstance(retval, idleFinish):
+                        del self._idlefuns[function]
+                        self.cooker.command.finishAsyncCommand(retval.msg)
+                        nextsleep = None
                     if retval is False:
                         del self._idlefuns[function]
                         nextsleep = None
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 07/15] process: Improve client disconnect/idle sync
  2022-12-29 17:07 [PATCH 00/15] Bitbake server thread enabling Richard Purdie
                   ` (5 preceding siblings ...)
  2022-12-29 17:07 ` [PATCH 06/15] server/process: Improve idle loop exit code Richard Purdie
@ 2022-12-29 17:07 ` Richard Purdie
  2022-12-29 17:07 ` [PATCH 08/15] process: Improve async command execution with idle interaction Richard Purdie
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Richard Purdie @ 2022-12-29 17:07 UTC (permalink / raw)
  To: bitbake-devel

When clients are disconnecting, we need to ensure things are in sync with
the idle thread. Use a threading event to show when things drop back to
idle (there is only one always present idle handler for inotify). Later
patches should be able to clean up the inotify handler to work differently
and avoid hardcoded values.

If we're not idle at client disconnect, shut down the server to solve
the dilemma. Not ideal but shouldn't happen much in practise and would be
restarted.

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

diff --git a/lib/bb/server/process.py b/lib/bb/server/process.py
index 3c909942aa..503d5dd686 100644
--- a/lib/bb/server/process.py
+++ b/lib/bb/server/process.py
@@ -94,6 +94,7 @@ class ProcessServer():
 
         self.idle = None
         self._idlefuns = {}
+        self.is_idle = threading.Event()
 
         self.bitbake_lock = lock
         self.bitbake_lock_name = lockname
@@ -173,6 +174,13 @@ class ProcessServer():
                 self.controllersock.close()
                 self.controllersock = False
             if self.haveui:
+                # Wait for the idle loop to have cleared (30s max)
+                self.is_idle.clear()
+                self.is_idle.wait(timeout=30)
+                if self.cooker.command.currentAsyncCommand is not None:
+                    serverlog("Idle loop didn't finish queued commands after 30s, exiting.")
+                    self.quit = True
+
                 fds.remove(self.command_channel)
                 bb.event.unregister_UIHhandler(self.event_handle, True)
                 self.command_channel_reply.writer.close()
@@ -184,7 +192,7 @@ class ProcessServer():
                 self.cooker.clientComplete()
                 self.haveui = False
             ready = select.select(fds,[],[],0)[0]
-            if newconnections:
+            if newconnections and not self.quit:
                 serverlog("Starting new client")
                 conn = newconnections.pop(-1)
                 fds.append(conn)
@@ -392,6 +400,10 @@ class ProcessServer():
                     del self._idlefuns[function]
                     self.quit = True
 
+            # FIXME - the 1 is the inotify processing in cooker which always runs
+            if len(self._idlefuns) <= 1:
+                self.is_idle.set()
+
             if nextsleep is not None:
                 select.select(fds,[],[],nextsleep)[0]
 
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 08/15] process: Improve async command execution with idle interaction
  2022-12-29 17:07 [PATCH 00/15] Bitbake server thread enabling Richard Purdie
                   ` (6 preceding siblings ...)
  2022-12-29 17:07 ` [PATCH 07/15] process: Improve client disconnect/idle sync Richard Purdie
@ 2022-12-29 17:07 ` Richard Purdie
  2022-12-29 17:07 ` [PATCH 09/15] knotty: Avoid looping with tracebacks Richard Purdie
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Richard Purdie @ 2022-12-29 17:07 UTC (permalink / raw)
  To: bitbake-devel

When running a new async command, the previous one might not have
finished executing in the idle handler. Add some code to use the event
logic to wait for the idle loop to finish for 30s before saying the
server is busy.

This avoids build failures when a UI quickly starts the next async
command before the code has finished shutdown/cleanup from the last one.

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

diff --git a/lib/bb/command.py b/lib/bb/command.py
index d03f35a896..41903241ab 100644
--- a/lib/bb/command.py
+++ b/lib/bb/command.py
@@ -60,7 +60,7 @@ class Command:
         # FIXME Add lock for this
         self.currentAsyncCommand = None
 
-    def runCommand(self, commandline, ro_only = False):
+    def runCommand(self, commandline, process_server, ro_only = False):
         command = commandline.pop(0)
 
         # Ensure cooker is ready for commands
@@ -100,7 +100,11 @@ class Command:
             else:
                 return result, None
         if self.currentAsyncCommand is not None:
-            return None, "Busy (%s in progress)" % self.currentAsyncCommand[0]
+            # Wait for the idle loop to have cleared (30s max)
+            process_server.is_idle.clear()
+            process_server.is_idle.wait(timeout=30)
+            if self.currentAsyncCommand is not None:
+                return None, "Busy (%s in progress)" % self.currentAsyncCommand[0]
         if command not in CommandsAsync.__dict__:
             return None, "No such command"
         self.currentAsyncCommand = (command, commandline)
diff --git a/lib/bb/server/process.py b/lib/bb/server/process.py
index 503d5dd686..fefc5a64fc 100644
--- a/lib/bb/server/process.py
+++ b/lib/bb/server/process.py
@@ -264,7 +264,7 @@ class ProcessServer():
                     continue
                 try:
                     serverlog("Running command %s" % command)
-                    self.command_channel_reply.send(self.cooker.command.runCommand(command))
+                    self.command_channel_reply.send(self.cooker.command.runCommand(command, self))
                     serverlog("Command Completed (socket: %s)" % os.path.exists(self.sockname))
                 except Exception as e:
                    stack = traceback.format_exc()
diff --git a/lib/bb/server/xmlrpcserver.py b/lib/bb/server/xmlrpcserver.py
index 01f55538ae..2e65dc34a9 100644
--- a/lib/bb/server/xmlrpcserver.py
+++ b/lib/bb/server/xmlrpcserver.py
@@ -118,7 +118,7 @@ class BitBakeXMLRPCServerCommands():
         """
         Run a cooker command on the server
         """
-        return self.server.cooker.command.runCommand(command, self.server.readonly)
+        return self.server.cooker.command.runCommand(command, self.server, self.server.readonly)
 
     def getEventHandle(self):
         return self.event_handle
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 09/15] knotty: Avoid looping with tracebacks
  2022-12-29 17:07 [PATCH 00/15] Bitbake server thread enabling Richard Purdie
                   ` (7 preceding siblings ...)
  2022-12-29 17:07 ` [PATCH 08/15] process: Improve async command execution with idle interaction Richard Purdie
@ 2022-12-29 17:07 ` Richard Purdie
  2022-12-29 17:07 ` [PATCH 10/15] event: Always use threadlock Richard Purdie
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Richard Purdie @ 2022-12-29 17:07 UTC (permalink / raw)
  To: bitbake-devel

If there are events queued and there is an exception in the main loop
of the UI code, it will print tracebacks on the console indefinitely.
Avoid that by improving the loop exit conditions.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/ui/knotty.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lib/bb/ui/knotty.py b/lib/bb/ui/knotty.py
index ab1a367be0..c398f591e8 100644
--- a/lib/bb/ui/knotty.py
+++ b/lib/bb/ui/knotty.py
@@ -631,7 +631,7 @@ def main(server, eventHandler, params, tf = TerminalFilter):
     termfilter = tf(main, helper, console_handlers, params.options.quiet)
     atexit.register(termfilter.finish)
 
-    while True:
+    while main.shutdown < 2:
         try:
             if (lastprint + printinterval) <= time.time():
                 termfilter.keepAlive(printinterval)
@@ -646,8 +646,6 @@ def main(server, eventHandler, params, tf = TerminalFilter):
                         return_value = 3
                         main.shutdown = 2
                     lastevent = time.time()
-                if main.shutdown > 1:
-                    break
                 if not parseprogress:
                     termfilter.updateFooter()
                 event = eventHandler.waitEvent(0.25)
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 10/15] event: Always use threadlock
  2022-12-29 17:07 [PATCH 00/15] Bitbake server thread enabling Richard Purdie
                   ` (8 preceding siblings ...)
  2022-12-29 17:07 ` [PATCH 09/15] knotty: Avoid looping with tracebacks Richard Purdie
@ 2022-12-29 17:07 ` Richard Purdie
  2022-12-29 17:07 ` [PATCH 11/15] server/process: Improve exception logging Richard Purdie
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Richard Purdie @ 2022-12-29 17:07 UTC (permalink / raw)
  To: bitbake-devel

With the move to a server idle thread, we always need threading. The
existing accessor functions could end up turning this off!

I was going to hold the lock whilst changing it, check if the value
was already set, cache the result and also fix the event code to always
release the lock with a try/finally.

Instead, disable the existing functions and use a with: block
to handle the lock, keeping things much simpler.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/event.py          | 73 +++++++++++++++++++---------------------
 lib/bb/server/process.py |  1 -
 lib/bb/tests/event.py    | 17 +---------
 3 files changed, 35 insertions(+), 56 deletions(-)

diff --git a/lib/bb/event.py b/lib/bb/event.py
index db90724444..7826541a64 100644
--- a/lib/bb/event.py
+++ b/lib/bb/event.py
@@ -68,16 +68,15 @@ _catchall_handlers = {}
 _eventfilter = None
 _uiready = False
 _thread_lock = threading.Lock()
-_thread_lock_enabled = False
 _heartbeat_enabled = False
 
 def enable_threadlock():
-    global _thread_lock_enabled
-    _thread_lock_enabled = True
+    # Always needed now
+    return
 
 def disable_threadlock():
-    global _thread_lock_enabled
-    _thread_lock_enabled = False
+    # Always needed now
+    return
 
 def enable_heartbeat():
     global _heartbeat_enabled
@@ -179,36 +178,30 @@ def print_ui_queue():
 
 def fire_ui_handlers(event, d):
     global _thread_lock
-    global _thread_lock_enabled
 
     if not _uiready:
         # No UI handlers registered yet, queue up the messages
         ui_queue.append(event)
         return
 
-    if _thread_lock_enabled:
-        _thread_lock.acquire()
-
-    errors = []
-    for h in _ui_handlers:
-        #print "Sending event %s" % event
-        try:
-             if not _ui_logfilters[h].filter(event):
-                 continue
-             # We use pickle here since it better handles object instances
-             # which xmlrpc's marshaller does not. Events *must* be serializable
-             # by pickle.
-             if hasattr(_ui_handlers[h].event, "sendpickle"):
-                _ui_handlers[h].event.sendpickle((pickle.dumps(event)))
-             else:
-                _ui_handlers[h].event.send(event)
-        except:
-            errors.append(h)
-    for h in errors:
-        del _ui_handlers[h]
-
-    if _thread_lock_enabled:
-        _thread_lock.release()
+    with _thread_lock:
+        errors = []
+        for h in _ui_handlers:
+            #print "Sending event %s" % event
+            try:
+                 if not _ui_logfilters[h].filter(event):
+                     continue
+                 # We use pickle here since it better handles object instances
+                 # which xmlrpc's marshaller does not. Events *must* be serializable
+                 # by pickle.
+                 if hasattr(_ui_handlers[h].event, "sendpickle"):
+                    _ui_handlers[h].event.sendpickle((pickle.dumps(event)))
+                 else:
+                    _ui_handlers[h].event.send(event)
+            except:
+                errors.append(h)
+        for h in errors:
+            del _ui_handlers[h]
 
 def fire(event, d):
     """Fire off an Event"""
@@ -322,21 +315,23 @@ def set_eventfilter(func):
     _eventfilter = func
 
 def register_UIHhandler(handler, mainui=False):
-    bb.event._ui_handler_seq = bb.event._ui_handler_seq + 1
-    _ui_handlers[_ui_handler_seq] = handler
-    level, debug_domains = bb.msg.constructLogOptions()
-    _ui_logfilters[_ui_handler_seq] = UIEventFilter(level, debug_domains)
-    if mainui:
-        global _uiready
-        _uiready = _ui_handler_seq
-    return _ui_handler_seq
+    with _thread_lock:
+        bb.event._ui_handler_seq = bb.event._ui_handler_seq + 1
+        _ui_handlers[_ui_handler_seq] = handler
+        level, debug_domains = bb.msg.constructLogOptions()
+        _ui_logfilters[_ui_handler_seq] = UIEventFilter(level, debug_domains)
+        if mainui:
+            global _uiready
+            _uiready = _ui_handler_seq
+        return _ui_handler_seq
 
 def unregister_UIHhandler(handlerNum, mainui=False):
     if mainui:
         global _uiready
         _uiready = False
-    if handlerNum in _ui_handlers:
-        del _ui_handlers[handlerNum]
+    with _thread_lock:
+        if handlerNum in _ui_handlers:
+            del _ui_handlers[handlerNum]
     return
 
 def get_uihandler():
diff --git a/lib/bb/server/process.py b/lib/bb/server/process.py
index fefc5a64fc..87b683ecf0 100644
--- a/lib/bb/server/process.py
+++ b/lib/bb/server/process.py
@@ -154,7 +154,6 @@ class ProcessServer():
         self.cooker.pre_serve()
 
         bb.utils.set_process_name("Cooker")
-        bb.event.enable_threadlock()
 
         ready = []
         newconnections = []
diff --git a/lib/bb/tests/event.py b/lib/bb/tests/event.py
index 4de4cced5e..d959f2d95d 100644
--- a/lib/bb/tests/event.py
+++ b/lib/bb/tests/event.py
@@ -451,10 +451,9 @@ class EventHandlingTest(unittest.TestCase):
             and disable threadlocks tests """
         bb.event.fire(bb.event.OperationStarted(), None)
 
-    def test_enable_threadlock(self):
+    def test_event_threadlock(self):
         """ Test enable_threadlock method """
         self._set_threadlock_test_mockups()
-        bb.event.enable_threadlock()
         self._set_and_run_threadlock_test_workers()
         # Calls to UI handlers should be in order as all the registered
         # handlers for the event coming from the first worker should be
@@ -462,20 +461,6 @@ class EventHandlingTest(unittest.TestCase):
         self.assertEqual(self._threadlock_test_calls,
                          ["w1_ui1", "w1_ui2", "w2_ui1", "w2_ui2"])
 
-
-    def test_disable_threadlock(self):
-        """ Test disable_threadlock method """
-        self._set_threadlock_test_mockups()
-        bb.event.disable_threadlock()
-        self._set_and_run_threadlock_test_workers()
-        # Calls to UI handlers should be intertwined together. Thanks to the
-        # delay in the registered handlers for the event coming from the first
-        # worker, the event coming from the second worker starts being
-        # processed before finishing handling the first worker event.
-        self.assertEqual(self._threadlock_test_calls,
-                         ["w1_ui1", "w2_ui1", "w1_ui2", "w2_ui2"])
-
-
 class EventClassesTest(unittest.TestCase):
     """ Event classes test class """
 
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 11/15] server/process: Improve exception logging
  2022-12-29 17:07 [PATCH 00/15] Bitbake server thread enabling Richard Purdie
                   ` (9 preceding siblings ...)
  2022-12-29 17:07 ` [PATCH 10/15] event: Always use threadlock Richard Purdie
@ 2022-12-29 17:07 ` Richard Purdie
  2022-12-29 17:07 ` [PATCH 12/15] cooker/cookerdata: Rework the way the datastores are reset Richard Purdie
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Richard Purdie @ 2022-12-29 17:07 UTC (permalink / raw)
  To: bitbake-devel

Currently if either idle functions loop suffers a traceback, it is
silently dropped and there is no log message to say what happened.
This change at least means the traceback is in the cooker log, making
some debugging possible.

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

diff --git a/lib/bb/server/process.py b/lib/bb/server/process.py
index 87b683ecf0..3038c63f55 100644
--- a/lib/bb/server/process.py
+++ b/lib/bb/server/process.py
@@ -397,6 +397,7 @@ class ProcessServer():
                     if not isinstance(exc, bb.BBHandledException):
                         logger.exception('Running idle function')
                     del self._idlefuns[function]
+                    serverlog("Exception %s broke the idle_thread, exiting" % traceback.format_exc())
                     self.quit = True
 
             # FIXME - the 1 is the inotify processing in cooker which always runs
@@ -430,6 +431,7 @@ class ProcessServer():
                 except Exception as exc:
                     if not isinstance(exc, bb.BBHandledException):
                         logger.exception('Running heartbeat function')
+                    serverlog("Exception %s broke in idle_commands, exiting" % traceback.format_exc())
                     self.quit = True
         if nextsleep and bb.event._heartbeat_enabled and now + nextsleep > self.next_heartbeat:
             # Shorten timeout so that we we wake up in time for
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 12/15] cooker/cookerdata: Rework the way the datastores are reset
  2022-12-29 17:07 [PATCH 00/15] Bitbake server thread enabling Richard Purdie
                   ` (10 preceding siblings ...)
  2022-12-29 17:07 ` [PATCH 11/15] server/process: Improve exception logging Richard Purdie
@ 2022-12-29 17:07 ` Richard Purdie
  2022-12-29 17:07 ` [PATCH 13/15] cooker: Ensure we clean up active idle handlers Richard Purdie
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Richard Purdie @ 2022-12-29 17:07 UTC (permalink / raw)
  To: bitbake-devel

As far as I could tell, the current code could result in some strange
situations where some data was set back to the original data store copy
but the multiconfig data was not restored. There are also some changes made
to the datastore which did not persist.

The data store was also being reset at every client reset, which seems
a little excessive if we can reset it to the original condition properly.

Move the __depends -> __base_depends rename into databuilder along with
the __bbclasstype change so these are saved in the original data.

Tweak the databuilder code to be clearer and easier to follow on which
copies are where, then save copies of all the mc datastores.

Finally, drop the cache invalidation upon reset for the base config
as we shouldn't need that now (oe-selftest -r tinfoil works with memory
resident bitbake which was the original reproducer requiring that change).

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/cooker.py     |  9 +++------
 lib/bb/cookerdata.py | 31 +++++++++++++++++++------------
 2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index 5e18c85bb8..ff7e48f6c0 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -409,9 +409,7 @@ class BBCooker:
             self.disableDataTracking()
 
         for mc in self.databuilder.mcdata.values():
-            mc.renameVar("__depends", "__base_depends")
             self.add_filewatch(mc.getVar("__base_depends", False), self.configwatcher)
-            mc.setVar("__bbclasstype", "recipe")
 
         self.baseconfig_valid = True
         self.parsecache_valid = False
@@ -445,10 +443,8 @@ class BBCooker:
                     upstream=upstream,
                 )
                 self.hashserv.serve_as_process()
-            self.data.setVar("BB_HASHSERVE", self.hashservaddr)
-            self.databuilder.origdata.setVar("BB_HASHSERVE", self.hashservaddr)
-            self.databuilder.data.setVar("BB_HASHSERVE", self.hashservaddr)
             for mc in self.databuilder.mcdata:
+                self.databuilder.mcorigdata[mc].setVar("BB_HASHSERVE", self.hashservaddr)
                 self.databuilder.mcdata[mc].setVar("BB_HASHSERVE", self.hashservaddr)
 
         bb.parse.init_parser(self.data)
@@ -1797,8 +1793,9 @@ class BBCooker:
         if hasattr(self, "data"):
            self.databuilder.reset()
            self.data = self.databuilder.data
+        # In theory tinfoil could have modified the base data before parsing,
+        # ideally need to track if anything did modify the datastore
         self.parsecache_valid = False
-        self.baseconfig_valid = False
 
 
 class CookerExit(bb.event.Event):
diff --git a/lib/bb/cookerdata.py b/lib/bb/cookerdata.py
index 650ae05ec9..b4e0c4216b 100644
--- a/lib/bb/cookerdata.py
+++ b/lib/bb/cookerdata.py
@@ -263,6 +263,7 @@ class CookerDataBuilder(object):
         self.mcdata = {}
 
     def parseBaseConfiguration(self, worker=False):
+        mcdata = {}
         data_hash = hashlib.sha256()
         try:
             self.data = self.parseConfigurationFiles(self.prefiles, self.postfiles)
@@ -288,18 +289,18 @@ class CookerDataBuilder(object):
 
             bb.parse.init_parser(self.data)
             data_hash.update(self.data.get_hash().encode('utf-8'))
-            self.mcdata[''] = self.data
+            mcdata[''] = self.data
 
             multiconfig = (self.data.getVar("BBMULTICONFIG") or "").split()
             for config in multiconfig:
                 if config[0].isdigit():
                     bb.fatal("Multiconfig name '%s' is invalid as multiconfigs cannot start with a digit" % config)
-                mcdata = self.parseConfigurationFiles(self.prefiles, self.postfiles, config)
-                bb.event.fire(bb.event.ConfigParsed(), mcdata)
-                self.mcdata[config] = mcdata
-                data_hash.update(mcdata.get_hash().encode('utf-8'))
+                parsed_mcdata = self.parseConfigurationFiles(self.prefiles, self.postfiles, config)
+                bb.event.fire(bb.event.ConfigParsed(), parsed_mcdata)
+                mcdata[config] = parsed_mcdata
+                data_hash.update(parsed_mcdata.get_hash().encode('utf-8'))
             if multiconfig:
-                bb.event.fire(bb.event.MultiConfigParsed(self.mcdata), self.data)
+                bb.event.fire(bb.event.MultiConfigParsed(mcdata), self.data)
 
             self.data_hash = data_hash.hexdigest()
         except (SyntaxError, bb.BBHandledException):
@@ -332,17 +333,23 @@ class CookerDataBuilder(object):
         if issues:
             raise bb.BBHandledException()
 
+        for mc in mcdata:
+            mcdata[mc].renameVar("__depends", "__base_depends")
+            mcdata[mc].setVar("__bbclasstype", "recipe")
+
         # Create a copy so we can reset at a later date when UIs disconnect
-        self.origdata = self.data
-        self.data = bb.data.createCopy(self.origdata)
-        self.mcdata[''] = self.data
+        self.mcorigdata = mcdata
+        for mc in mcdata:
+            self.mcdata[mc] = bb.data.createCopy(mcdata[mc])
+        self.data = self.mcdata['']
 
     def reset(self):
         # We may not have run parseBaseConfiguration() yet
-        if not hasattr(self, 'origdata'):
+        if not hasattr(self, 'mcorigdata'):
             return
-        self.data = bb.data.createCopy(self.origdata)
-        self.mcdata[''] = self.data
+        for mc in self.mcorigdata:
+            self.mcdata[mc] = bb.data.createCopy(self.mcorigdata[mc])
+        self.data = self.mcdata['']
 
     def _findLayerConf(self, data):
         return findConfigFile("bblayers.conf", data)
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 13/15] cooker: Ensure we clean up active idle handlers
  2022-12-29 17:07 [PATCH 00/15] Bitbake server thread enabling Richard Purdie
                   ` (11 preceding siblings ...)
  2022-12-29 17:07 ` [PATCH 12/15] cooker/cookerdata: Rework the way the datastores are reset Richard Purdie
@ 2022-12-29 17:07 ` Richard Purdie
  2022-12-29 17:07 ` [PATCH 14/15] cache: Drop reciever side counting for SiggenRecipeInfo Richard Purdie
  2022-12-29 17:07 ` [PATCH 15/15] server/process: Add debug to show which handlers are active Richard Purdie
  14 siblings, 0 replies; 18+ messages in thread
From: Richard Purdie @ 2022-12-29 17:07 UTC (permalink / raw)
  To: bitbake-devel

When the cooker shutdown method is called, ensure we clean up any
active idle handlers before we exit as these could have trees of
processes in them and these need to be notified and reaped.

Ensure at actual shutdown when the idle loop has been terminated
we don't trigger the idle loop call though as it will no longer
be active at that point.

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

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index ff7e48f6c0..a5a635858c 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -149,7 +149,7 @@ class BBCooker:
     Manages one bitbake build run
     """
 
-    def __init__(self, featureSet=None, idleCallBackRegister=None):
+    def __init__(self, featureSet=None, idleCallBackRegister=None, waitIdle=None):
         self.recipecaches = None
         self.eventlog = None
         self.skiplist = {}
@@ -164,6 +164,7 @@ class BBCooker:
         self.configuration = bb.cookerdata.CookerConfiguration()
 
         self.idleCallBackRegister = idleCallBackRegister
+        self.waitIdle = waitIdle
 
         bb.debug(1, "BBCooker starting %s" % time.time())
         sys.stdout.flush()
@@ -1753,7 +1754,7 @@ class BBCooker:
         return
 
     def post_serve(self):
-        self.shutdown(force=True)
+        self.shutdown(force=True, idle=False)
         prserv.serv.auto_shutdown()
         if hasattr(bb.parse, "siggen"):
             bb.parse.siggen.exit()
@@ -1763,12 +1764,15 @@ class BBCooker:
         if hasattr(self, "data"):
             bb.event.fire(CookerExit(), self.data)
 
-    def shutdown(self, force = False):
+    def shutdown(self, force=False, idle=True):
         if force:
             self.state = state.forceshutdown
         else:
             self.state = state.shutdown
 
+        if idle:
+            self.waitIdle(30)
+
         if self.parser:
             self.parser.shutdown(clean=not force)
             self.parser.final_cleanup()
diff --git a/lib/bb/server/process.py b/lib/bb/server/process.py
index 3038c63f55..365758f85a 100644
--- a/lib/bb/server/process.py
+++ b/lib/bb/server/process.py
@@ -150,6 +150,11 @@ class ProcessServer():
 
         return ret
 
+    def wait_for_idle(self, timeout=30):
+        # Wait for the idle loop to have cleared (30s max)
+        self.is_idle.clear()
+        self.is_idle.wait(timeout=timeout)
+
     def main(self):
         self.cooker.pre_serve()
 
@@ -174,8 +179,7 @@ class ProcessServer():
                 self.controllersock = False
             if self.haveui:
                 # Wait for the idle loop to have cleared (30s max)
-                self.is_idle.clear()
-                self.is_idle.wait(timeout=30)
+                self.wait_for_idle(30)
                 if self.cooker.command.currentAsyncCommand is not None:
                     serverlog("Idle loop didn't finish queued commands after 30s, exiting.")
                     self.quit = True
@@ -309,7 +313,7 @@ class ProcessServer():
         self.sock.close()
 
         try:
-            self.cooker.shutdown(True)
+            self.cooker.shutdown(True, idle=False)
             self.cooker.notifier.stop()
             self.cooker.confignotifier.stop()
         except:
@@ -607,7 +611,7 @@ def execServer(lockfd, readypipeinfd, lockname, sockname, server_timeout, xmlrpc
         writer = ConnectionWriter(readypipeinfd)
         try:
             featureset = []
-            cooker = bb.cooker.BBCooker(featureset, server.register_idle_function)
+            cooker = bb.cooker.BBCooker(featureset, server.register_idle_function, server.wait_for_idle)
             cooker.configuration.profile = profile
         except bb.BBHandledException:
             return None
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 14/15] cache: Drop reciever side counting for SiggenRecipeInfo
  2022-12-29 17:07 [PATCH 00/15] Bitbake server thread enabling Richard Purdie
                   ` (12 preceding siblings ...)
  2022-12-29 17:07 ` [PATCH 13/15] cooker: Ensure we clean up active idle handlers Richard Purdie
@ 2022-12-29 17:07 ` Richard Purdie
  2022-12-30 15:46   ` [bitbake-devel] " Peter Kjellerstedt
  2022-12-29 17:07 ` [PATCH 15/15] server/process: Add debug to show which handlers are active Richard Purdie
  14 siblings, 1 reply; 18+ messages in thread
From: Richard Purdie @ 2022-12-29 17:07 UTC (permalink / raw)
  To: bitbake-devel

Joshua Watt pointed out maintaining the counting on both sides of the
connection isn't needed. Remove the receiver side counting and simplify
the code, also allowing errors if the counts do go out of sync.

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

diff --git a/lib/bb/cache.py b/lib/bb/cache.py
index 3fc097241a..ee924b2d2b 100644
--- a/lib/bb/cache.py
+++ b/lib/bb/cache.py
@@ -285,7 +285,6 @@ class SiggenRecipeInfo(RecipeInfoCommon):
         cls.save_map = {}
         cls.save_count = 1
         cls.restore_map = {}
-        cls.restore_count = {}
 
     @classmethod
     def _save(cls, deps):
@@ -294,11 +293,13 @@ class SiggenRecipeInfo(RecipeInfoCommon):
             return deps
         for dep in deps:
             fs = deps[dep]
-            if fs in cls.save_map:
+            if fs is None:
+                ret.append((dep, None, None))
+            elif fs in cls.save_map:
                 ret.append((dep, None, cls.save_map[fs]))
             else:
                 cls.save_map[fs] = cls.save_count
-                ret.append((dep, fs, None))
+                ret.append((dep, fs, cls.save_count))
                 cls.save_count = cls.save_count + 1
         return ret
 
@@ -309,18 +310,18 @@ class SiggenRecipeInfo(RecipeInfoCommon):
             return deps
         if pid not in cls.restore_map:
             cls.restore_map[pid] = {}
-            cls.restore_count[pid] = 1
         map = cls.restore_map[pid]
         for dep, fs, mapnum in deps:
-            if mapnum:
+            if fs is None and mapnum is None:
+                ret[dep] = None
+            elif fs is None:
                 ret[dep] = map[mapnum]
             else:
                 try:
                     fs = cls.store[fs]
                 except KeyError:
                     cls.store[fs] = fs
-                map[cls.restore_count[pid]] = fs
-                cls.restore_count[pid] = cls.restore_count[pid] + 1
+                map[mapnum] = fs
                 ret[dep] = fs
         return ret
 
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 15/15] server/process: Add debug to show which handlers are active
  2022-12-29 17:07 [PATCH 00/15] Bitbake server thread enabling Richard Purdie
                   ` (13 preceding siblings ...)
  2022-12-29 17:07 ` [PATCH 14/15] cache: Drop reciever side counting for SiggenRecipeInfo Richard Purdie
@ 2022-12-29 17:07 ` Richard Purdie
  14 siblings, 0 replies; 18+ messages in thread
From: Richard Purdie @ 2022-12-29 17:07 UTC (permalink / raw)
  To: bitbake-devel

Add some logging to show when handlers are added/removed to allow
a better idea of what the server code is doing from the server log
file.

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

diff --git a/lib/bb/server/process.py b/lib/bb/server/process.py
index 365758f85a..b69432ead2 100644
--- a/lib/bb/server/process.py
+++ b/lib/bb/server/process.py
@@ -112,6 +112,7 @@ class ProcessServer():
         """Register a function to be called while the server is idle"""
         assert hasattr(function, '__call__')
         self._idlefuns[function] = data
+        serverlog("Registering idle function %s" % str(function))
 
     def run(self):
 
@@ -380,10 +381,12 @@ class ProcessServer():
                 try:
                     retval = function(self, data, False)
                     if isinstance(retval, idleFinish):
+                        serverlog("Removing idle function %s at idleFinish" % str(function))
                         del self._idlefuns[function]
                         self.cooker.command.finishAsyncCommand(retval.msg)
                         nextsleep = None
                     if retval is False:
+                        serverlog("Removing idle function %s" % str(function))
                         del self._idlefuns[function]
                         nextsleep = None
                     elif retval is True:
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* RE: [bitbake-devel] [PATCH 03/15] knotty: Ping the server/cooker periodically
  2022-12-29 17:07 ` [PATCH 03/15] knotty: Ping the server/cooker periodically Richard Purdie
@ 2022-12-30 15:44   ` Peter Kjellerstedt
  0 siblings, 0 replies; 18+ messages in thread
From: Peter Kjellerstedt @ 2022-12-30 15:44 UTC (permalink / raw)
  To: Richard Purdie, bitbake-devel

> -----Original Message-----
> From: bitbake-devel@lists.openembedded.org <bitbake-devel@lists.openembedded.org> On Behalf Of Richard Purdie
> Sent: den 29 december 2022 18:07
> To: bitbake-devel@lists.openembedded.org
> Subject: [bitbake-devel] [PATCH 03/15] knotty: Ping the server/cooker periodically
> 
> We've seeing failures where the UI hangs if the server disappears. Ping

Change " We've seeing" to either "We're seeing" or "We've seen".

> the cooker/server if we've not had any events in the last minute so we can
> check if it is still alive.
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  lib/bb/ui/knotty.py | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/bb/ui/knotty.py b/lib/bb/ui/knotty.py
> index 61cf0a37f4..ab1a367be0 100644
> --- a/lib/bb/ui/knotty.py
> +++ b/lib/bb/ui/knotty.py
> @@ -625,7 +625,8 @@ def main(server, eventHandler, params, tf = TerminalFilter):
> 
>      printintervaldelta = 10 * 60 # 10 minutes
>      printinterval = printintervaldelta
> -    lastprint = time.time()
> +    pinginterval = 1 * 60 # 1 minutes

Change "1 minutes" to "1 minute".

> +    lastevent = lastprint = time.time()
> 
>      termfilter = tf(main, helper, console_handlers, params.options.quiet)
>      atexit.register(termfilter.finish)
> @@ -637,6 +638,14 @@ def main(server, eventHandler, params, tf = TerminalFilter):
>                  printinterval += printintervaldelta
>              event = eventHandler.waitEvent(0)
>              if event is None:
> +                if (lastevent + pinginterval) <= time.time():
> +                    ret, error = server.runCommand(["ping"])
> +                    if error or not ret:
> +                        termfilter.clearFooter()
> +                        print("No reply after pinging server (%s, %s), exiting." % (str(error), str(ret)))
> +                        return_value = 3
> +                        main.shutdown = 2
> +                    lastevent = time.time()
>                  if main.shutdown > 1:
>                      break
>                  if not parseprogress:
> @@ -644,6 +653,7 @@ def main(server, eventHandler, params, tf = TerminalFilter):
>                  event = eventHandler.waitEvent(0.25)
>                  if event is None:
>                      continue
> +            lastevent = time.time()
>              helper.eventHandler(event)
>              if isinstance(event, bb.runqueue.runQueueExitWait):
>                  if not main.shutdown:
> --
> 2.37.2

//Peter



^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [bitbake-devel] [PATCH 14/15] cache: Drop reciever side counting for SiggenRecipeInfo
  2022-12-29 17:07 ` [PATCH 14/15] cache: Drop reciever side counting for SiggenRecipeInfo Richard Purdie
@ 2022-12-30 15:46   ` Peter Kjellerstedt
  0 siblings, 0 replies; 18+ messages in thread
From: Peter Kjellerstedt @ 2022-12-30 15:46 UTC (permalink / raw)
  To: Richard Purdie, bitbake-devel

> -----Original Message-----
> From: bitbake-devel@lists.openembedded.org <bitbake-devel@lists.openembedded.org> On Behalf Of Richard Purdie
> Sent: den 29 december 2022 18:07
> To: bitbake-devel@lists.openembedded.org
> Subject: [bitbake-devel] [PATCH 14/15] cache: Drop reciever side counting for SiggenRecipeInfo

Change "reciever" to "receiver".

> 
> Joshua Watt pointed out maintaining the counting on both sides of the
> connection isn't needed. Remove the receiver side counting and simplify
> the code, also allowing errors if the counts do go out of sync.
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  lib/bb/cache.py | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/bb/cache.py b/lib/bb/cache.py
> index 3fc097241a..ee924b2d2b 100644
> --- a/lib/bb/cache.py
> +++ b/lib/bb/cache.py
> @@ -285,7 +285,6 @@ class SiggenRecipeInfo(RecipeInfoCommon):
>          cls.save_map = {}
>          cls.save_count = 1
>          cls.restore_map = {}
> -        cls.restore_count = {}
> 
>      @classmethod
>      def _save(cls, deps):
> @@ -294,11 +293,13 @@ class SiggenRecipeInfo(RecipeInfoCommon):
>              return deps
>          for dep in deps:
>              fs = deps[dep]
> -            if fs in cls.save_map:
> +            if fs is None:
> +                ret.append((dep, None, None))
> +            elif fs in cls.save_map:
>                  ret.append((dep, None, cls.save_map[fs]))
>              else:
>                  cls.save_map[fs] = cls.save_count
> -                ret.append((dep, fs, None))
> +                ret.append((dep, fs, cls.save_count))
>                  cls.save_count = cls.save_count + 1
>          return ret
> 
> @@ -309,18 +310,18 @@ class SiggenRecipeInfo(RecipeInfoCommon):
>              return deps
>          if pid not in cls.restore_map:
>              cls.restore_map[pid] = {}
> -            cls.restore_count[pid] = 1
>          map = cls.restore_map[pid]
>          for dep, fs, mapnum in deps:
> -            if mapnum:
> +            if fs is None and mapnum is None:
> +                ret[dep] = None
> +            elif fs is None:
>                  ret[dep] = map[mapnum]
>              else:
>                  try:
>                      fs = cls.store[fs]
>                  except KeyError:
>                      cls.store[fs] = fs
> -                map[cls.restore_count[pid]] = fs
> -                cls.restore_count[pid] = cls.restore_count[pid] + 1
> +                map[mapnum] = fs
>                  ret[dep] = fs
>          return ret
> 
> --
> 2.37.2

//Peter



^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-12-30 15:46 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-29 17:07 [PATCH 00/15] Bitbake server thread enabling Richard Purdie
2022-12-29 17:07 ` [PATCH 01/15] command: Tweak finishAsyncCommand ordering to avoid races Richard Purdie
2022-12-29 17:07 ` [PATCH 02/15] cooker: Ensure commands clean up any parser processes Richard Purdie
2022-12-29 17:07 ` [PATCH 03/15] knotty: Ping the server/cooker periodically Richard Purdie
2022-12-30 15:44   ` [bitbake-devel] " Peter Kjellerstedt
2022-12-29 17:07 ` [PATCH 04/15] event: Add enable/disable heartbeat code Richard Purdie
2022-12-29 17:07 ` [PATCH 05/15] server/process: Run idle commands in a separate idle thread Richard Purdie
2022-12-29 17:07 ` [PATCH 06/15] server/process: Improve idle loop exit code Richard Purdie
2022-12-29 17:07 ` [PATCH 07/15] process: Improve client disconnect/idle sync Richard Purdie
2022-12-29 17:07 ` [PATCH 08/15] process: Improve async command execution with idle interaction Richard Purdie
2022-12-29 17:07 ` [PATCH 09/15] knotty: Avoid looping with tracebacks Richard Purdie
2022-12-29 17:07 ` [PATCH 10/15] event: Always use threadlock Richard Purdie
2022-12-29 17:07 ` [PATCH 11/15] server/process: Improve exception logging Richard Purdie
2022-12-29 17:07 ` [PATCH 12/15] cooker/cookerdata: Rework the way the datastores are reset Richard Purdie
2022-12-29 17:07 ` [PATCH 13/15] cooker: Ensure we clean up active idle handlers Richard Purdie
2022-12-29 17:07 ` [PATCH 14/15] cache: Drop reciever side counting for SiggenRecipeInfo Richard Purdie
2022-12-30 15:46   ` [bitbake-devel] " Peter Kjellerstedt
2022-12-29 17:07 ` [PATCH 15/15] server/process: Add debug to show which handlers are active Richard Purdie

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.