/usr/bin/script is broken in HEAD by r205008, in 8-STABLE by 205635. After these changes it no longer exits with the same exit code as the command it runs (thus, if the command fails, it'll still exit with 0) Among other things, this breaks portupgrade, as it now mistakingly interprets failed port builds as successfull ones, thus removing ports without installing newer versions. How-To-Repeat: /usr/bin/script from r205007 # script -q /dev/null true; echo $? 0 # script -q /dev/null false; echo $? 1 /usr/bin/script from r205008 # script -q /dev/null true; echo $? 0 # script -q /dev/null false; echo $? 0
* Dmitry Marakasov <amdmi3@FreeBSD.org> wrote: > After these changes it no longer exits with the same exit code as the command it runs (thus, if the command fails, it'll still exit with 0) > Among other things, this breaks portupgrade, as it now mistakingly interprets failed port builds as successfull ones, thus removing ports without installing newer versions. Looks like my fix is correct, but it did expose a new bug inside script(1). We must not use WNOHANG in wait3(), because we must now really wait for the child process to properly shut down. Index: script.c =================================================================== --- script.c (revision 207446) +++ script.c (working copy) @@ -223,7 +223,7 @@ int die, e, status; die = e = 0; - while ((pid = wait3(&status, WNOHANG, 0)) > 0) + while ((pid = wait3(&status, 0, 0)) > 0) if (pid == child) { die = 1; if (WIFEXITED(status)) -- Ed Schouten <ed@80386.nl> WWW: http://80386.nl/
This seems to work, thanks! -- Dmitry Marakasov . 55B5 0596 FF1E 8D84 5F56 9510 D35A 80DD F9D2 F77D amdmi3@amdmi3.ru ..: jabber: amdmi3@jabber.ru http://www.amdmi3.ru
Author: ed Date: Fri Apr 30 22:33:49 2010 New Revision: 207453 URL: http://svn.freebsd.org/changeset/base/207453 Log: Remove WNOHANG flag from wait3(). Because script(1) now reliably terminates when the TTY is closed, it may be the case that the call to wait3() occurs just before the child process exits. This causes error codes to be ignored. Just change script(1) to use waitpid() instead of wait3(). This makes it more portable and prevents the need for a loop, since waitpid() only returns a specified process. PR: bin/146189 Tested by: amdmi3@, older version MFC after: 2 weeks Modified: head/usr.bin/script/script.c Modified: head/usr.bin/script/script.c ============================================================================== --- head/usr.bin/script/script.c Fri Apr 30 22:31:37 2010 (r207452) +++ head/usr.bin/script/script.c Fri Apr 30 22:33:49 2010 (r207453) @@ -219,23 +219,17 @@ usage(void) void finish(void) { - pid_t pid; - int die, e, status; + int e, status; - die = e = 0; - while ((pid = wait3(&status, WNOHANG, 0)) > 0) - if (pid == child) { - die = 1; - if (WIFEXITED(status)) - e = WEXITSTATUS(status); - else if (WIFSIGNALED(status)) - e = WTERMSIG(status); - else /* can't happen */ - e = 1; - } - - if (die) + if (waitpid(child, &status, 0) == child) { + if (WIFEXITED(status)) + e = WEXITSTATUS(status); + else if (WIFSIGNALED(status)) + e = WTERMSIG(status); + else /* can't happen */ + e = 1; done(e); + } } void _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
State Changed From-To: open->patched
Author: ed Date: Fri May 14 12:34:06 2010 New Revision: 208070 URL: http://svn.freebsd.org/changeset/base/208070 Log: MFC r207453: Remove WNOHANG flag from wait3(). Because script(1) now reliably terminates when the TTY is closed, it may be the case that the call to wait3() occurs just before the child process exits. This causes error codes to be ignored. Just change script(1) to use waitpid() instead of wait3(). This makes it more portable and prevents the need for a loop, since waitpid() only returns a specified process. PR: bin/146189 Tested by: amdmi3@, older version Modified: stable/8/usr.bin/script/script.c Directory Properties: stable/8/usr.bin/script/ (props changed) Modified: stable/8/usr.bin/script/script.c ============================================================================== --- stable/8/usr.bin/script/script.c Fri May 14 10:06:20 2010 (r208069) +++ stable/8/usr.bin/script/script.c Fri May 14 12:34:06 2010 (r208070) @@ -219,23 +219,17 @@ usage(void) void finish(void) { - pid_t pid; - int die, e, status; + int e, status; - die = e = 0; - while ((pid = wait3(&status, WNOHANG, 0)) > 0) - if (pid == child) { - die = 1; - if (WIFEXITED(status)) - e = WEXITSTATUS(status); - else if (WIFSIGNALED(status)) - e = WTERMSIG(status); - else /* can't happen */ - e = 1; - } - - if (die) + if (waitpid(child, &status, 0) == child) { + if (WIFEXITED(status)) + e = WEXITSTATUS(status); + else if (WIFSIGNALED(status)) + e = WTERMSIG(status); + else /* can't happen */ + e = 1; done(e); + } } void _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
State Changed From-To: patched->closed Fix has been merged to stable/8.
script(1) is also showing the same incorrect behavior on 7.4 and 7-STABLE. I believe that the root cause identified in this PR is wrong, as r205008 was not MFCd to 7-STABLE. However, the fix from this PR (r207453) does fix it for 7.4/7-STABLE as well. Without this fix, some automated scripts depending on the return status of script(1) will improperly continue, or fail. portupgrade(1) is prone to this. See ports/131111 and ports/147242. Regards, Bryan Drewery