[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#779608: xvfb: cleanup on signal exits as well



Jérémy Lal:
> 2016-03-20 15:58 GMT+01:00 Ximin Luo <infinity0@debian.org>:
>> I couldn't figure out why it is run twice [*], but if you just change "rm
>> -r" to "rm -rf" it will no longer fail. I don't see any problem with that.
> 
> I suggest instead to keep a "cleaned" global variable to track if the
> cleanup function has run once before or not.
> 
>> [..]
> 
> I've had exactly the same problem with a totally unrelated program
> recently. I suppose that behavior is explained somewhere in the F.M.
> 

I found an answer here: https://stackoverflow.com/questions/8122779/is-it-necessary-to-specify-traps-other-than-exit

Basically for trapped signals (and not untrapped signals) dash will further execute the EXIT handler. So a solution is to clear the handler at the end of clean_up, which I've done in the updated patch. You should be able to verify this yourself by adding 'echo "clean_up"' to the start of that function.

Bash has a different behaviour, and possibly this patch wasn't necessary back when everyone was using bash for /bin/sh.

(I still think it's worth using "rm -rf" instead of "rm -r", we're cleaning up and we don't really care if it's already been deleted.)

X

-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
git://github.com/infinity0/pubkeys.git
--- /usr/bin/xvfb-run.orig	2016-03-20 15:53:08.182925279 +0100
+++ /usr/bin/xvfb-run	2016-03-20 16:26:01.260548596 +0100
@@ -82,7 +82,7 @@
         XAUTHORITY=$AUTHFILE xauth remove ":$SERVERNUM" >>"$ERRORFILE" 2>&1
     fi
     if [ -n "$XVFB_RUN_TMPDIR" ]; then
-        if ! rm -r "$XVFB_RUN_TMPDIR"; then
+        if ! rm -rf "$XVFB_RUN_TMPDIR"; then
             error "problem while cleaning up temporary directory"
             exit 5
         fi
@@ -90,6 +90,7 @@
     if [ -n "$XVFBPID" ]; then
         kill "$XVFBPID" >>"$ERRORFILE" 2>&1
     fi
+    trap - EXIT
 }
 
 # Parse the command line.
@@ -140,7 +141,7 @@
 fi
 
 # tidy up after ourselves
-trap clean_up EXIT
+trap clean_up EXIT HUP INT QUIT PIPE ALRM TERM
 
 # If the user did not specify an X authorization file to use, set up a temporary
 # directory to house one.
@@ -165,7 +166,7 @@
     XVFBPID=$!
 
     wait || :
-    if kill -0 $XVFBPID 2>/dev/null; then
+    if kill -0 "$XVFBPID" 2>/dev/null; then
         break
     elif [ -n "$AUTONUM" ]; then
         # The display is in use so try another one (if '-a' was specified).
@@ -180,7 +181,7 @@
 
 # Start the command and save its exit status.
 set +e
-DISPLAY=:$SERVERNUM XAUTHORITY=$AUTHFILE "$@" 2>&1
+DISPLAY=:$SERVERNUM XAUTHORITY=$AUTHFILE "$@" 2>&1 & wait "$!"
 RETVAL=$?
 set -e
 

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: