NetBSD Problem Report #55605

From www@netbsd.org  Mon Aug 24 22:51:32 2020
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 7DF0D1A923A
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 24 Aug 2020 22:51:32 +0000 (UTC)
Message-Id: <20200824225131.5B85C1A923D@mollari.NetBSD.org>
Date: Mon, 24 Aug 2020 22:51:31 +0000 (UTC)
From: soumendra@tamu.edu
Reply-To: soumendra@tamu.edu
To: gnats-bugs@NetBSD.org
Subject: [PATCH] script(1): does not wait on child pid when stdin is not a tty
X-Send-Pr-Version: www-1.0

>Number:         55605
>Category:       bin
>Synopsis:       [PATCH] script(1): does not wait on child pid when stdin is not a tty
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    bin-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Aug 24 22:55:00 +0000 2020
>Closed-Date:    Wed Jun 09 04:07:27 +0000 2021
>Last-Modified:  Wed Jun 09 04:07:27 +0000 2021
>Originator:     Soumendra Ganguly
>Release:        9.0
>Organization:
Texas A&M University
>Environment:
NetBSD localhost 9.0 NetBSD 9.0 (GENERIC) #0: Fri Feb 14 00:06:28 UTC 2020  mkrepro@mkrepro.NetBSD.org:/usr/src/sys/arch/amd64/compile/GENERIC amd64
>Description:
Parent process does not call SIGCHLD handler when stdin is not a tty. It does not wait on the child pid. It simply calls done() and exits. This leads to strange output on screen. See the "how to repeat the problem" section for more details. Patch is included.
>How-To-Repeat:
Create regular file "~/tmp" with following contents:

ls
exit
<newline at end of file >

Now, call script < ~/tmp to notice the following:

"Script done, output file is typescript" comes right after the message
"Script started, output file is typescript" and before the pty echo and the data written to stdout. Add a "printf("SIGCHLD\n");" to finish() and run again to notice that actually finish() [ the SIGCHLD handler ] is never called by the parent. It is called by the child when it receives SIGCHLD when the subchild exits.

>Fix:
--- src/usr.bin/script/script.c	2020-08-24 17:27:30.907579625 -0500
+++ script.c	2020-08-24 17:29:00.950890312 -0500
@@ -204,7 +204,7 @@
 			record(fscript, ibuf, cc, 'i');
 		(void)write(master, ibuf, cc);
 	}
-	done();
+	waitpid(-1, NULL, 0); /* Dummy wait. wait3() called upon SIGCHLD. */
 	/* NOTREACHED */
 	return EXIT_SUCCESS;
 }
@@ -249,7 +249,7 @@
 		if (scc <= 0)
 			break;
 		cc = (size_t)scc;
-		(void)write(1, obuf, cc);
+		(void)write(STDOUT_FILENO, obuf, cc);
 		if (rawout)
 			record(fscript, obuf, cc, 'o');
 		else
@@ -258,7 +258,7 @@
 		if (flush)
 			(void)fflush(fscript);
 	}
-	done();
+	waitpid(-1, NULL, 0); /* Dummy wait. wait3() called upon SIGCHLD. */
 }

 static void

>Release-Note:

>Audit-Trail:
From: Soumendra Ganguly <soumendra@tamu.edu>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: bin/55605: [PATCH] script(1): does not wait on child pid when
 stdin is not a tty
Date: Mon, 24 Aug 2020 19:02:11 -0500

 I made a slight modification to the patch. Since finish() calls
 wait3() anyway, I decided to remove the ambiguous waitpid()s that I
 added [ not sure if they will succeed or EINTR out due to SIGCHLD ]
 and replace them with simple while loops. The next message will
 contain the modified patch [ inline ].

 On 8/24/20, gnats-admin@netbsd.org <gnats-admin@netbsd.org> wrote:
 > Thank you very much for your problem report.
 > It has the internal identification `bin/55605'.
 > The individual assigned to look at your
 > report is: bin-bug-people.
 >
 >>Category:       bin
 >>Responsible:    bin-bug-people
 >>Synopsis:       [PATCH] script(1): does not wait on child pid when stdin is
 >> not a tty
 >>Arrival-Date:   Mon Aug 24 22:55:00 +0000 2020
 >
 >

From: Soumendra Ganguly <soumendra@tamu.edu>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: bin/55605: [PATCH] script(1): does not wait on child pid when
 stdin is not a tty
Date: Mon, 24 Aug 2020 19:03:23 -0500

 --- src/usr.bin/script/script.c	2020-08-24 17:27:30.907579625 -0500
 +++ script.c	2020-08-24 18:51:36.715164374 -0500
 @@ -204,7 +204,8 @@
  			record(fscript, ibuf, cc, 'i');
  		(void)write(master, ibuf, cc);
  	}
 -	done();
 +	while (1) /* Wait for SIGCHLD. */
 +		sleep(1);
  	/* NOTREACHED */
  	return EXIT_SUCCESS;
  }
 @@ -249,7 +250,7 @@
  		if (scc <= 0)
  			break;
  		cc = (size_t)scc;
 -		(void)write(1, obuf, cc);
 +		(void)write(STDOUT_FILENO, obuf, cc);
  		if (rawout)
  			record(fscript, obuf, cc, 'o');
  		else
 @@ -258,7 +259,8 @@
  		if (flush)
  			(void)fflush(fscript);
  	}
 -	done();
 +	while (1) /* Wait for SIGCHLD. */
 +		sleep(1);
  }

  static void


 On 8/24/20, Soumendra Ganguly <soumendra@tamu.edu> wrote:
 > I made a slight modification to the patch. Since finish() calls
 > wait3() anyway, I decided to remove the ambiguous waitpid()s that I
 > added [ not sure if they will succeed or EINTR out due to SIGCHLD ]
 > and replace them with simple while loops. The next message will
 > contain the modified patch [ inline ].
 >
 > On 8/24/20, gnats-admin@netbsd.org <gnats-admin@netbsd.org> wrote:
 >> Thank you very much for your problem report.
 >> It has the internal identification `bin/55605'.
 >> The individual assigned to look at your
 >> report is: bin-bug-people.
 >>
 >>>Category:       bin
 >>>Responsible:    bin-bug-people
 >>>Synopsis:       [PATCH] script(1): does not wait on child pid when stdin
 >>> is
 >>> not a tty
 >>>Arrival-Date:   Mon Aug 24 22:55:00 +0000 2020
 >>
 >>
 >

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/55605 CVS commit: src/usr.bin/script
Date: Fri, 28 Aug 2020 13:10:15 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Fri Aug 28 17:10:15 UTC 2020

 Modified Files:
 	src/usr.bin/script: script.c

 Log Message:
 PR/55605: Soumendra Ganguly: Wait for child to finish when stdin is not a tty


 To generate a diff of this commit:
 cvs rdiff -u -r1.26 -r1.27 src/usr.bin/script/script.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Soumendra Ganguly <soumendra@tamu.edu>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: PR/55605 CVS commit: src/usr.bin/script
Date: Sun, 30 Aug 2020 08:34:42 -0500

 Thank you for the commit. sigsuspend instead of the while loop was brilliant.

 -Soumendra

 On 8/28/20, Christos Zoulas <christos@netbsd.org> wrote:
 > The following reply was made to PR bin/55605; it has been noted by GNATS.
 >
 > From: "Christos Zoulas" <christos@netbsd.org>
 > To: gnats-bugs@gnats.NetBSD.org
 > Cc:
 > Subject: PR/55605 CVS commit: src/usr.bin/script
 > Date: Fri, 28 Aug 2020 13:10:15 -0400
 >
 >  Module Name:	src
 >  Committed By:	christos
 >  Date:		Fri Aug 28 17:10:15 UTC 2020
 >
 >  Modified Files:
 >  	src/usr.bin/script: script.c
 >
 >  Log Message:
 >  PR/55605: Soumendra Ganguly: Wait for child to finish when stdin is not a
 > tty
 >
 >
 >  To generate a diff of this commit:
 >  cvs rdiff -u -r1.26 -r1.27 src/usr.bin/script/script.c
 >
 >  Please note that diffs are not public domain; they are subject to the
 >  copyright notices on the relevant files.
 >
 >

State-Changed-From-To: open->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Wed, 09 Jun 2021 04:07:27 +0000
State-Changed-Why:
Christos committed it.


>Unformatted:

Home	PR Database Search