strace signal handling regression

[Jan Kratochvil]

On 2007-09-21 22:07, Dmitry V. Levin wrote:
> There seems to be signal handling regression introduced at 2007-06-11 by commit
> http://strace.cvs.sourceforge.net/strace/strace/strace.c#rev1.77
> 
> Test case for uninteruptable strace follows.
> 
> $ strace -qfF -e trace=none -o '|/bin/cat' sleep 10 & for i in `seq 1 9`; do sleep 1 && kill %+ || break; done
> [1] 2345
> |/bin/cat: Broken pipe
> |/bin/cat: Broken pipe
> |/bin/cat: Broken pipe
> |/bin/cat: Broken pipe
> |/bin/cat: Broken pipe
> |/bin/cat: Broken pipe
> |/bin/cat: Broken pipe
> |/bin/cat: Broken pipe
> $ |/bin/cat: Broken pipe
> [1]+ Done strace -qfF -e trace=none -o '|/bin/cat' sleep 10

The regression affects runs with spawned command (no `-p') while using `-o'.
Confirming it is a defect due to my committed patch you referenced above.

Problem is setting up the uninterruptibility signal handlers before the child
process gets spawned.  The spawned process inherits the signals settings.

Formerly the signals were setup only after all of the either spawning or
attaching work has been done.  Still it is important to setup the
uninterruptibility signal handlers before the attaching is done to avoid
leaving pending SIGSTOP resulting from PTRACE_ATTACH uncaught.


The INTERACTIVE flag meaning is unclear for me for me but this patch restores
the original interruptible vs. uninterruptible behavior.  The PFLAG_SEEN flag
should be IMO enough for the purposes the INTERACTIVE flag is currently in use.
To be patched separately.


Thanks for finding the regression,
Jan
-------------- next part --------------
2007-11-03  Jan Kratochvil  <jan.kratochvil at redhat.com>

	* strace.c (main): Move the STARTUP_CHILD call before setting up the
	signal handlers.  New comment about the valid internal states.

--- strace.c	8 Oct 2007 21:04:41 -0000	1.83
+++ strace.c	3 Nov 2007 10:46:10 -0000
@@ -817,6 +817,20 @@ main(int argc, char *argv[])
 		interactive = 0;
 		qflag = 1;
 	}
+	/* Valid states here:
+	   optind < argc	pflag_seen	outfname	interactive
+	   1			0		0		1
+	   0			1		0		1
+	   1			0		1		0
+	   0			1		1		1
+	 */
+
+	/* STARTUP_CHILD must be called before the signal handlers get
+	   installed below as they are inherited into the spawned process.
+	   Also we do not need to be protected by them as during interruption
+	   in the STARTUP_CHILD mode we kill the spawned process anyway.  */
+	if (!pflag_seen)
+		startup_child(&argv[optind]);
 
 	sigemptyset(&empty_set);
 	sigemptyset(&blocked_set);
@@ -857,8 +871,6 @@ main(int argc, char *argv[])
 
 	if (pflag_seen)
 		startup_attach();
-	else
-		startup_child(&argv[optind]);
 
 	if (trace() < 0)
 		exit(1);