[gentoo-portage-dev] Re: [PATCH] bin/ebuild-ipc.py: use threading if available

[Zac Medico]

This updated patch is fixed to prevent ebuild-ipc processes from
hanging around in some cases like bug #345455:

	https://bugs.gentoo.org/show_bug.cgi?id=345455

I'll be maintaining this patch in the following branch:

	https://github.com/zmedico/portage/tree/bug_524328

From 5e37cd413b8c18830cd378dfe85cc3211cebe636 Mon Sep 17 00:00:00 2001
From: Zac Medico <zmedico@g.o>
Date: Tue, 7 Oct 2014 21:52:08 -0700
Subject: [PATCH] bin/ebuild-ipc.py: use threading if available

If threading is available then use a thread instead of a fork for
writing to the fifo. This has the advantage of avoiding a possible
python futex deadlock following fork as reported in bug #524328.

X-Gentoo-Bug: 524328
X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=524328
---
 bin/ebuild-ipc.py | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/bin/ebuild-ipc.py b/bin/ebuild-ipc.py
index 3e6d90c..9963b5c 100755
--- a/bin/ebuild-ipc.py
+++ b/bin/ebuild-ipc.py
@@ -5,6 +5,7 @@
 # This is a helper which ebuild processes can use
 # to communicate with portage's main python process.
 
+import dummy_threading
 import logging
 import os
 import pickle
@@ -13,6 +14,11 @@ import signal
 import sys
 import time
 
+try:
+	import threading
+except ImportError:
+	threading = dummy_threading
+
 def debug_signal(signum, frame):
 	import pdb
 	pdb.set_trace()
@@ -46,9 +52,10 @@ portage._disable_legacy_globals()
 
 from portage.util._async.ForkProcess import ForkProcess
 from portage.util._eventloop.global_event_loop import global_event_loop
+from _emerge.AbstractPollTask import AbstractPollTask
 from _emerge.PipeReader import PipeReader
 
-class FifoWriter(ForkProcess):
+class FifoWriterFork(ForkProcess):
 
 	__slots__ = ('buf', 'fifo',)
 
@@ -58,6 +65,44 @@ class FifoWriter(ForkProcess):
 			f.write(self.buf)
 		return os.EX_OK
 
+class FifoWriterThread(AbstractPollTask):
+
+	__slots__ = ('buf', 'fifo', '_thread',)
+
+	def _start(self):
+		self._registered = True
+		self._thread = threading.Thread(target=self._write_fifo)
+		self._thread.daemon = True
+		self._thread.start()
+
+	def _write_fifo(self):
+		with open(self.fifo, 'wb', 0) as f:
+			f.write(self.buf)
+		# Thread-safe callback to EventLoop
+		self.scheduler.timeout_add(0, self._exit_cb, os.EX_OK)
+
+	def _exit_cb(self, returncode):
+		if self.returncode is None:
+			self.returncode = returncode
+			self._unregister()
+			self.wait()
+		return False
+
+	def _cancel(self):
+		# Do not join the thread, since it could block indefinitely
+		# when opening/writing the fifo (as in bug #345455). Since
+		# there's no way to force thread termination, simply allow
+		# the daemon thread to terminate when the main thread
+		# terminates. This class is only instantiated once, so there
+		# will be no more than a single thread writing to self.fifo
+		# during the life of the program (no chance of multiple
+		# threads interfering with eachother).
+		self.scheduler.timeout_add(0,
+			self._exit_cb, -signal.SIGINT)
+
+	def _unregister(self):
+		self._registered = False
+
 class EbuildIpc(object):
 
 	# Timeout for each individual communication attempt (we retry
@@ -210,6 +255,12 @@ class EbuildIpc(object):
 		# considerations. This helps to avoid possible race conditions
 		# from interference between timeouts and blocking IO operations.
 		msg = portage.localization._('during write')
+		if threading is dummy_threading:
+			# Try to avoid this fork due to possible
+			# python deadlock (bug #524328).
+			FifoWriter = FifoWriterFork
+		else:
+			FifoWriter = FifoWriterThread
 		retval = self._run_writer(FifoWriter(buf=pickle.dumps(args),
 			fifo=self.ipc_in_fifo, scheduler=global_event_loop()), msg)
 
-- 
1.8.5.5