[bitbake][dunfell][1.46][PATCH 0/4] Patch review

[bitbake][dunfell][1.46][PATCH 0/4] Patch review

[Steve Sakoman]

Martin Jansa proposed this patch set on the list a couple of weeks ago
and there was a bit of discussion on list and on IRC:

https://lists.openembedded.org/g/bitbake-devel/message/12366

I've had this patch set in my testing queue since then and I haven't
encountered any issues on the autobuilder or with local testing.

Most recent a-full test results here:

https://autobuilder.yoctoproject.org/typhoon/#/builders/83/builds/2247

If you have any comments please respond by end of day Thursday.

The following changes since commit 078f3164dcb1de7a141bec3a8fd52631d0362631:

  providers: selected version not available should be a warning (2021-05-13 04:26:15 -1000)

are available in the Git repository at:

  git://git.openembedded.org/bitbake-contrib stable/1.46-nut
  http://cgit.openembedded.org/bitbake-contrib/log/?h=stable/1.46-nut

Richard Purdie (4):
  cooker: Ensure parse_quit thread is closed down
  cooker: Explictly shut down the sync thread
  cooker: Ensure parser is cleaned up
  cooker: Avoid parser deadlocks

 lib/bb/cooker.py | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

-- 
2.25.1

[bitbake][dunfell][1.46][PATCH 1/4] cooker: Ensure parse_quit thread is closed down

[Steve Sakoman]

From: Richard Purdie <richard.purdie@linuxfoundation.org>

Each run through the parser would leak a thread from the queue created to
shut the parser down. Close this down correctly and clean up the code flow
slightly whilst in the area, making sure this thread does shut down correctly
(we don't care if it loses data).

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
Signed-off-by: Steve Sakoman <steve@sakoman.com>
---
 lib/bb/cooker.py | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index 11cc2b95..4820d268 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -2056,12 +2056,11 @@ class CookerParser(object):
                                             self.total)
 
             bb.event.fire(event, self.cfgdata)
-            for process in self.processes:
-                self.parser_quit.put(None)
-        else:
-            self.parser_quit.cancel_join_thread()
-            for process in self.processes:
-                self.parser_quit.put(None)
+
+        # Allow data left in the cancel queue to be discarded
+        self.parser_quit.cancel_join_thread()
+        for process in self.processes:
+            self.parser_quit.put(None)
 
         # Cleanup the queue before call process.join(), otherwise there might be
         # deadlocks.
@@ -2078,6 +2077,9 @@ class CookerParser(object):
             else:
                 process.join()
 
+        self.parser_quit.close()
+        self.parser_quit.join_thread()
+
         sync = threading.Thread(target=self.bb_cache.sync)
         sync.start()
         multiprocessing.util.Finalize(None, sync.join, exitpriority=-100)
-- 
2.25.1

[bitbake][dunfell][1.46][PATCH 2/4] cooker: Explictly shut down the sync thread

[Steve Sakoman]

From: Richard Purdie <richard.purdie@linuxfoundation.org>

Hongxu Jia reported a problem where the bb_cache files were not always being
written out correctly. This was due to the sync thread being terminated
prematurely.

Whilst the preceeding changes mean the exit handler for this thread is now
correctly called since we switch to using sys.exit() instead of os._exit(),
this write can happen after we drop the bitbake lock, leading to potential
races. Avoid that headache by adding in explicit thread join() calls before
we drop the lock (which atexit or Finalize can't do).

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
Signed-off-by: Steve Sakoman <steve@sakoman.com>
---
 lib/bb/cooker.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index 4820d268..87884574 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -1650,6 +1650,7 @@ class BBCooker:
 
         if self.parser:
             self.parser.shutdown(clean=not force, force=force)
+            self.parser.final_cleanup()
 
     def finishcommand(self):
         self.state = state.initial
@@ -2015,6 +2016,7 @@ class CookerParser(object):
 
         self.start()
         self.haveshutdown = False
+        self.syncthread = None
 
     def start(self):
         self.results = self.load_cached()
@@ -2081,8 +2083,8 @@ class CookerParser(object):
         self.parser_quit.join_thread()
 
         sync = threading.Thread(target=self.bb_cache.sync)
+        self.syncthread = sync
         sync.start()
-        multiprocessing.util.Finalize(None, sync.join, exitpriority=-100)
         bb.codeparser.parser_cache_savemerge()
         bb.fetch.fetcher_parse_done()
         if self.cooker.configuration.profile:
@@ -2096,6 +2098,10 @@ class CookerParser(object):
             bb.utils.process_profilelog(profiles, pout = pout)
             print("Processed parsing statistics saved to %s" % (pout))
 
+    def final_cleanup(self):
+        if self.syncthread:
+            self.syncthread.join()
+
     def load_cached(self):
         for filename, appends in self.fromcache:
             cached, infos = self.bb_cache.load(filename, appends)
-- 
2.25.1

[bitbake][dunfell][1.46][PATCH 3/4] cooker: Ensure parser is cleaned up

[Steve Sakoman]

From: Richard Purdie <richard.purdie@linuxfoundation.org>

During cooker shutdown, its possible the parser isn't cleaned up. Fix
this (which may partially explain why threads were left hanging around
at exit).

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
Signed-off-by: Steve Sakoman <steve@sakoman.com>
---
 lib/bb/cooker.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index 87884574..4d2c62cd 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -1636,6 +1636,7 @@ class BBCooker:
         return
 
     def post_serve(self):
+        self.shutdown(force=True)
         prserv.serv.auto_shutdown()
         if self.hashserv:
             self.hashserv.process.terminate()
-- 
2.25.1

[bitbake][dunfell][1.46][PATCH 4/4] cooker: Avoid parser deadlocks

[Steve Sakoman]

From: Richard Purdie <richard.purdie@linuxfoundation.org>

If you make parsing fail (e.g. add something like:

X := "${@d.getVar('MCMACHINES').split()[1]}"

to meson.bbclass, then run "while true; do bitbake -g bash; done"
it will eventually hang. It appears the cancel_join_thread() call the
parsing failure triggers, breaks the results_queue badly enough that it
sits in read() indefintely (called from self.result_queue.get(timeout=0.25)).
The timeout only applies to lock aquisition, not the read call.

I've tried various other approaches such as using cancel_join_thread()
in other places but the only way things don't lock up is to avoid
cancel_join_thread() entirely for results_queue.

I do have a concern that this may adversely affect Ctrl+C handling
but equally, its broken now already and this appears to improve
things.

[YOCTO #14034]

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

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index 4d2c62cd..730cdc56 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -1933,7 +1933,8 @@ class Parser(multiprocessing.Process):
             except queue.Empty:
                 pass
             else:
-                self.results.cancel_join_thread()
+                self.results.close()
+                self.results.join_thread()
                 break
 
             if pending:
@@ -1942,6 +1943,8 @@ class Parser(multiprocessing.Process):
                 try:
                     job = self.jobs.pop()
                 except IndexError:
+                    self.results.close()
+                    self.results.join_thread()
                     break
                 result = self.parse(*job)
                 # Clear the siggen cache after parsing to control memory usage, its huge
@@ -2060,8 +2063,6 @@ class CookerParser(object):
 
             bb.event.fire(event, self.cfgdata)
 
-        # Allow data left in the cancel queue to be discarded
-        self.parser_quit.cancel_join_thread()
         for process in self.processes:
             self.parser_quit.put(None)
 
@@ -2081,7 +2082,8 @@ class CookerParser(object):
                 process.join()
 
         self.parser_quit.close()
-        self.parser_quit.join_thread()
+        # Allow data left in the cancel queue to be discarded
+        self.parser_quit.cancel_join_thread()
 
         sync = threading.Thread(target=self.bb_cache.sync)
         self.syncthread = sync
-- 
2.25.1