Skip to contentSkip to conversation

process: Improve exit handling and hangs

PATCH In review Richard Purdie 2 participants 3 messages Aug 24, 2013 – Aug 27, 2013

lore
Share & tools
lore

Richard Purdie

author
#0 lore raw

PATCH 1/1 process: Improve exit handling and hangs

It turns out we have a number of different ways the process server termination can hang. If we call cancel_join_thread() on the event queue, it means that it can be left containing partial data. This means the reading of the event queue in the terminate() function can hang, the timeout and block parameters to Queue.get() don't make any difference.

Equally, if we don't call cancel_join_thread(), the join_thread in terminate() will hang giving a different deadlock.

The best solution I could find is to loop over the process is_alive() after requesting it stops, trying to join the thread and if that fails, try and flush the event queue again.

It wasn't clear what difference a force option should make in this case, we're gracefully trying to empty queues and shut down regardless of whether its a SIGTERM so I've simply removed the force option.

Signed-off-by Richard Purdie <[email protected]>

---

Jason: Not sure if this or the other patch will help the hang you are seeing or not but they seem like good changes regardless and fix real world issues.

bitbake/lib/bb/server/process.py +17 -19
index 0d4a26c..99a6bf5 100644
@@ -105,7 +105,7 @@ class ProcessServer(Process, BaseImplServer):
             except Exception:
                 logger.exception('Running command %s', command)
 
-        self.event_queue.cancel_join_thread()
+        self.event_queue.close()
         bb.event.unregister_UIHhandler(self.event_handle)
         self.command_channel.close()
         self.cooker.stop()

@@ -150,27 +150,25 @@ class BitBakeProcessServerConnection(BitBakeBaseServerConnection):
         self.connection = ServerCommunicator(self.ui_channel)
         self.events = self.event_queue
 
-    def terminate(self, force = False):
+    def terminate(self):
+        def flushevents():
+            while True:
+                try:
+                    event = self.event_queue.get(block=False)
+                except (Empty, IOError):
+                    break
+                if isinstance(event, logging.LogRecord):
+                    logger.handle(event)
+
         signal.signal(signal.SIGINT, signal.SIG_IGN)
         self.procserver.stop()
-        if force:
-            self.procserver.join(0.5)
-            if self.procserver.is_alive():
-                self.procserver.terminate()
-                self.procserver.join()
-        else:
-            self.procserver.join()
-        while True:
-            try:
-                event = self.event_queue.get(block=False)
-            except (Empty, IOError):
-                break
-            if isinstance(event, logging.LogRecord):
-                logger.handle(event)
+
+        while self.procserver.is_alive():
+            flushevents()
+            self.procserver.join(0.1)
+
         self.ui_channel.close()
         self.event_queue.close()
-        if force:
-            sys.exit(1)
 
 # Wrap Queue to provide API which isn't server implementation specific
 class ProcessEventQueue(multiprocessing.queues.Queue):

@@ -203,5 +201,5 @@ class BitBakeServer(BitBakeBaseServer):
 
     def establishConnection(self):
         self.connection = BitBakeProcessServerConnection(self.serverImpl, self.ui_channel, self.event_queue)
-        signal.signal(signal.SIGTERM, lambda i, s: self.connection.terminate(force=True))
+        signal.signal(signal.SIGTERM, lambda i, s: self.connection.terminate())
         return self.connection
author
#1 lore raw

bitbake/lib/bb/server/process.py class BitBakeServer(BitBakeBaseServer): · line 205

Quoted 7 lines
bitbake/lib/bb/server/process.py +1 -1
@@ -203,5 +201,5 @@ class BitBakeServer(BitBakeBaseServer):
 
     def establishConnection(self):
         self.connection = BitBakeProcessServerConnection(self.serverImpl, self.ui_channel, self.event_queue)
-        signal.signal(signal.SIGTERM, lambda i, s: self.connection.terminate(force=True))
+        signal.signal(signal.SIGTERM, lambda i, s: self.connection.terminate())
         return self.connection

FWIW I think this piece of the change *may* make bitbake's handling of Ctrl+C more robust. I know people have reported problems with that and the function being called here was full of deadlocks. I'd be interested in feedback on whether it helps.

Cheers,

Richard

#2 lore raw
In reply to Richard Purdie
Quoted 17 lines · Richard Purdie

It turns out we have a number of different ways the process server termination can hang. If we call cancel_join_thread() on the event queue, it means that it can be left containing partial data. This means the reading of the event queue in the terminate() function can hang, the timeout and block parameters to Queue.get() don't make any difference.

Equally, if we don't call cancel_join_thread(), the join_thread in terminate() will hang giving a different deadlock.

The best solution I could find is to loop over the process is_alive() after requesting it stops, trying to join the thread and if that fails, try and flush the event queue again.

It wasn't clear what difference a force option should make in this case, we're gracefully trying to empty queues and shut down regardless of whether its a SIGTERM so I've simply removed the force option.

Quoted 6 lines
Signed-off-by Richard Purdie <[email protected]>

---

Jason: Not sure if this or the other patch will help the hang you are seeing or not but they seem like good changes regardless and fix real world issues.

Certainly the behavior is a bit better with your additional patches, but it is not the root of the problem. I have tested your patches in the heavy load situations where we have observed all the hangs.

I'll send a patch separately along with an explanation of the root cause of the hangs in the PR Server.

As for your patches, I have reviewed and tested them: Acked-by: Jason Wessel <[email protected]>

Cheers,
Jason.