diff mbox series

[03/10] event: builtins fix for 'd' deletion

Message ID 20221222234726.579702-3-richard.purdie@linuxfoundation.org
State Accepted, archived
Commit 1e12f0a4b592dacd006d370ec29cd71d2a44312e
Headers show
Series [01/10] knotty: Ping the server/cooker periodically | expand

Commit Message

Richard Purdie Dec. 22, 2022, 11:47 p.m. UTC
I've been seeing event handlers where 'd' seems to disappear half way through
event handler execution. This is problematic when multiple threads are active
since this code assumes single threading.

The easiest fix is to change the handler function calls to contain d as a
parameter as we do elsewhere for other functions. This will break any non-text
handlers but I was only able to spot one of those in runqueue. It will also
break handlers than call functions that assume 'd' is in the global namespace
but those failures should be obvious and we can fix those to pass d around.

This solution avoids manipulating builtins which was always a horrible thing
to do anyway and solves the issue without needing locking, thankfully.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 lib/bb/event.py       | 18 ++++--------------
 lib/bb/runqueue.py    |  2 +-
 lib/bb/tests/color.py |  2 +-
 lib/bb/tests/event.py | 21 +++++++++++----------
 4 files changed, 17 insertions(+), 26 deletions(-)
diff mbox series

Patch

diff --git a/lib/bb/event.py b/lib/bb/event.py
index 21e9a1b025..db90724444 100644
--- a/lib/bb/event.py
+++ b/lib/bb/event.py
@@ -71,11 +71,6 @@  _thread_lock = threading.Lock()
 _thread_lock_enabled = False
 _heartbeat_enabled = False
 
-if hasattr(__builtins__, '__setitem__'):
-    builtins = __builtins__
-else:
-    builtins = __builtins__.__dict__
-
 def enable_threadlock():
     global _thread_lock_enabled
     _thread_lock_enabled = True
@@ -94,12 +89,8 @@  def disable_heartbeat():
 
 def execute_handler(name, handler, event, d):
     event.data = d
-    addedd = False
-    if 'd' not in builtins:
-        builtins['d'] = d
-        addedd = True
     try:
-        ret = handler(event)
+        ret = handler(event, d)
     except (bb.parse.SkipRecipe, bb.BBHandledException):
         raise
     except Exception:
@@ -113,8 +104,7 @@  def execute_handler(name, handler, event, d):
         raise
     finally:
         del event.data
-        if addedd:
-            del builtins['d']
+
 
 def fire_class_handlers(event, d):
     if isinstance(event, logging.LogRecord):
@@ -262,12 +252,12 @@  def register(name, handler, mask=None, filename=None, lineno=None, data=None):
     if handler is not None:
         # handle string containing python code
         if isinstance(handler, str):
-            tmp = "def %s(e):\n%s" % (name, handler)
+            tmp = "def %s(e, d):\n%s" % (name, handler)
             try:
                 code = bb.methodpool.compile_cache(tmp)
                 if not code:
                     if filename is None:
-                        filename = "%s(e)" % name
+                        filename = "%s(e, d)" % name
                     code = compile(tmp, filename, "exec", ast.PyCF_ONLY_AST)
                     if lineno is not None:
                         ast.increment_lineno(code, lineno-1)
diff --git a/lib/bb/runqueue.py b/lib/bb/runqueue.py
index b9dd830b31..ce711b6252 100644
--- a/lib/bb/runqueue.py
+++ b/lib/bb/runqueue.py
@@ -1511,7 +1511,7 @@  class RunQueue:
 
             if not self.dm_event_handler_registered:
                  res = bb.event.register(self.dm_event_handler_name,
-                                         lambda x: self.dm.check(self) if self.state in [runQueueRunning, runQueueCleanUp] else False,
+                                         lambda x, y: self.dm.check(self) if self.state in [runQueueRunning, runQueueCleanUp] else False,
                                          ('bb.event.HeartbeatEvent',), data=self.cfgData)
                  self.dm_event_handler_registered = True
 
diff --git a/lib/bb/tests/color.py b/lib/bb/tests/color.py
index 88dd278006..bb70cb393d 100644
--- a/lib/bb/tests/color.py
+++ b/lib/bb/tests/color.py
@@ -20,7 +20,7 @@  class ProgressWatcher:
     def __init__(self):
         self._reports = []
 
-    def handle_event(self, event):
+    def handle_event(self, event, d):
         self._reports.append((event.progress, event.rate))
 
     def reports(self):
diff --git a/lib/bb/tests/event.py b/lib/bb/tests/event.py
index 9ca7e9bc8e..4de4cced5e 100644
--- a/lib/bb/tests/event.py
+++ b/lib/bb/tests/event.py
@@ -157,7 +157,7 @@  class EventHandlingTest(unittest.TestCase):
                                  self._test_process.event_handler,
                                  event,
                                  None)
-        self._test_process.event_handler.assert_called_once_with(event)
+        self._test_process.event_handler.assert_called_once_with(event, None)
 
     def test_fire_class_handlers(self):
         """ Test fire_class_handlers method """
@@ -175,10 +175,10 @@  class EventHandlingTest(unittest.TestCase):
         bb.event.fire_class_handlers(event1, None)
         bb.event.fire_class_handlers(event2, None)
         bb.event.fire_class_handlers(event2, None)
-        expected_event_handler1 = [call(event1)]
-        expected_event_handler2 = [call(event1),
-                                   call(event2),
-                                   call(event2)]
+        expected_event_handler1 = [call(event1, None)]
+        expected_event_handler2 = [call(event1, None),
+                                   call(event2, None),
+                                   call(event2, None)]
         self.assertEqual(self._test_process.event_handler1.call_args_list,
                          expected_event_handler1)
         self.assertEqual(self._test_process.event_handler2.call_args_list,
@@ -205,7 +205,7 @@  class EventHandlingTest(unittest.TestCase):
         bb.event.fire_class_handlers(event2, None)
         bb.event.fire_class_handlers(event2, None)
         expected_event_handler1 = []
-        expected_event_handler2 = [call(event1)]
+        expected_event_handler2 = [call(event1, None)]
         self.assertEqual(self._test_process.event_handler1.call_args_list,
                          expected_event_handler1)
         self.assertEqual(self._test_process.event_handler2.call_args_list,
@@ -223,7 +223,7 @@  class EventHandlingTest(unittest.TestCase):
         self.assertEqual(result, bb.event.Registered)
         bb.event.fire_class_handlers(event1, None)
         bb.event.fire_class_handlers(event2, None)
-        expected = [call(event1), call(event2)]
+        expected = [call(event1, None), call(event2, None)]
         self.assertEqual(self._test_process.event_handler1.call_args_list,
                          expected)
 
@@ -237,7 +237,7 @@  class EventHandlingTest(unittest.TestCase):
         self.assertEqual(result, bb.event.Registered)
         bb.event.fire_class_handlers(event1, None)
         bb.event.fire_class_handlers(event2, None)
-        expected = [call(event1), call(event2), call(event1)]
+        expected = [call(event1, None), call(event2, None), call(event1, None)]
         self.assertEqual(self._test_process.event_handler1.call_args_list,
                          expected)
 
@@ -251,7 +251,7 @@  class EventHandlingTest(unittest.TestCase):
         self.assertEqual(result, bb.event.Registered)
         bb.event.fire_class_handlers(event1, None)
         bb.event.fire_class_handlers(event2, None)
-        expected = [call(event1), call(event2), call(event1), call(event2)]
+        expected = [call(event1,None), call(event2, None), call(event1, None), call(event2, None)]
         self.assertEqual(self._test_process.event_handler1.call_args_list,
                          expected)
 
@@ -359,9 +359,10 @@  class EventHandlingTest(unittest.TestCase):
 
         event1 = bb.event.ConfigParsed()
         bb.event.fire(event1, None)
-        expected = [call(event1)]
+        expected = [call(event1, None)]
         self.assertEqual(self._test_process.event_handler1.call_args_list,
                          expected)
+        expected = [call(event1)]
         self.assertEqual(self._test_ui1.event.send.call_args_list,
                          expected)