I am also regularily annoyed by this problem, which seems to be too easy
to trigger simply by running unattended-upgrades.
Arthur de Jong has found the explanation: Whenever run-parts (with the
--report option) starts a script which leaks stdout or stderr, then
run-parts will hang forever in select().
This simple leaky script will demonstrate the issue:
#!/bin/sh
# closing stdin and stdout, but leaking stderr...
(exec setsid sleep 5 <&- >&-) &
echo $0 is exiting now
exit 0
run-parts --report will wait until sleep finishes when running this
script. If the sleep is replaced by a daemon started by apt, or some
other permanently running application, then run-parts will wait forever.
Now, I will absolutely agree that the descriptor leak is a bug in the
package started by apt, but I also agree with Arthur de Jong that
run-parts could be smarter. The fact that run-parts will hang only if
started with "--report" is an indication that this should be considered a
bug in run-parts as well, IMHO. Using "--report" should not have side
effects.
I have attached a primitive attempt to fix this. Not sure if this is
entirely safe or not, but it's a starting point.... signalfd() would
have been perfect, but it's not very portable.
Bjørn
>From 6dd8546883b0d13d5b2c41d2d94097ce40c4a84a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B8rn=20Mork?= <bj...@mork.no>
Date: Mon, 21 Mar 2011 11:15:34 +0100
Subject: [PATCH] Close report pipes if child exits. closes: #379645
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Bjørn Mork <bj...@mork.no>
---
run-parts.c | 31 ++++++++++++++++++-------------
1 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/run-parts.c b/run-parts.c
index 8278cd9..eb03d39 100644
--- a/run-parts.c
+++ b/run-parts.c
@@ -208,19 +208,17 @@ void run_part(char *progname)
while (pout[0] >= 0 || perr[0] >= 0) {
- do {
- FD_ZERO(&set);
- if (pout[0] >= 0)
- FD_SET(pout[0], &set);
- if (perr[0] >= 0)
- FD_SET(perr[0], &set);
- r = select(max, &set, 0, 0, 0);
- } while (r < 0 && errno == EINTR);
-
- if (r < 0) {
- /* assert(errno != EINTR) */
- error("select: %s", strerror(errno));
- exit(1);
+ FD_ZERO(&set);
+ if (pout[0] >= 0)
+ FD_SET(pout[0], &set);
+ if (perr[0] >= 0)
+ FD_SET(perr[0], &set);
+ r = select(max, &set, 0, 0, 0);
+
+ if (r < 0 && errno != EINTR) {
+ /* assert(errno != EINTR) */
+ error("select: %s", strerror(errno));
+ exit(1);
}
else if (r > 0) {
if (pout[0] >= 0 && FD_ISSET(pout[0], &set)) {
@@ -268,6 +266,13 @@ void run_part(char *progname)
/* assert(FALSE): select was called with infinite timeout, so
it either returns successfully or is interrupted */
} /*if */
+ /* close pipe if child exited */
+ if (waitpid(pid, &result, WNOHANG)) {
+ close(pout[0]);
+ pout[0] = -1;
+ close(perr[0]);
+ perr[0] = -1;
+ }
} /*while */
}
--
1.7.2.5
