Re: Bash: sleep execution issue with bash loadable builtins
Hi,
Thanks a lot for your review comments.
I have reworked on the bash sleep fix based on your suggestion about signal
and trap handling in fsleep( ).
I have attached the fix patch for your kind reference.
Test logs with fix patch:
==
root@x86-generic-64:~/bash-4.2# ./bash
root@x86-generic-64:~/bash-4.2# cd examples/loadables/
root@x86-generic-64:~/bash-4.2/examples/loadables# enable -n ./sleep sleep
bash: enable: ./sleep: not a shell builtin
bash: enable: sleep: not a shell builtin
root@x86-generic-64:~/bash-4.2/examples/loadables# enable -f ./sleep sleep
root@x86-generic-64:~/bash-4.2/examples/loadables# date; sleep 1 & date; sleep
10; date
Tue Nov 28 07:18:03 UTC 2017
[1]+ Donesleep 1
[1] 19524
Tue Nov 28 07:18:03 UTC 2017
Tue Nov 28 07:18:13 UTC 2017
[1]+ Donesleep 1
root@x86-generic-64:~/bash-4.2/examples/loadables#
root@x86-generic-64:~/bash-4.2/examples/loadables#
root@x86-generic-64:~/bash-4.2/examples/loadables# date; sleep 10 & date; sleep
4; date
Tue Nov 28 07:18:25 UTC 2017
[1] 19528
Tue Nov 28 07:18:25 UTC 2017
Tue Nov 28 07:18:29 UTC 2017
root@x86-generic-64:~/bash-4.2/examples/loadables#
<->
Please kindly review and suggest your comments. you suggestions and
comments helps to enhance the fix
and submit the same to bash gnu mainline.
Thanks,
Thiruvadi Rajaraman
On Tue, Nov 28, 2017 at 2:09 AM, Chet Ramey wrote:
> On 11/27/17 4:17 AM, Thiruvadi Rajaraman wrote:
> > Hi,
> >
> > Found a 'sleep' execution issue with bash loadable builtins and has
> > performed
> > the below sleep test on bash-4.4-rc1.
>
> That's an interesting one. It looks like the SIGCHLD interrupts the select
> loop, even though bash supplies SA_RESTART when installing its SIGCHLD
> handler. It's probably too hard to restart it in general, since select
> doesn't necessarily modify its timeval argument when it returns early
> (Linux does; many other OSs do not).
>
> There is a problem with your fix in that, in most cases, you've just made
> everything that uses this function non-interruptible, especially in an
> interactive shell. I think a better fix would be to change fsleep() to cope
> with select(2) being interrupted using the bash primitives that deal with
> signal and trap handling.
>
> Chet
> --
> ``The lyf so short, the craft so long to lerne.'' - Chaucer
> ``Ars longa, vita brevis'' - Hippocrates
> Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~
> chet/
>
Fix for bash loadable builtin sleep issue
The rootcause of the sleep execution failure has found with
the SIGCHLD affects the execution of select().
An issue fixed by ignoring the SIGCHLD signal during the
select() execution.
Signed-off-by: Thiruvadi Rajaraman
Index: bash-4.2/lib/sh/ufuncs.c
===
--- bash-4.2.orig/lib/sh/ufuncs.c
+++ bash-4.2/lib/sh/ufuncs.c
@@ -19,7 +19,7 @@
*/
#include "config.h"
-
+#include
#include "bashtypes.h"
#if defined (TIME_WITH_SYS_TIME)
@@ -86,6 +86,7 @@ fsleep(sec, usec)
unsigned int sec, usec;
{
struct timeval tv;
+ signal(SIGCHLD, NULL);
tv.tv_sec = sec;
tv.tv_usec = usec;
回复：[here string] uncompatible change on here string function
Thanks all.
Now I got it:
The point is the action on bash-4.2 and before deosn't match the action on ksh
and zsh.
Jianhong
发自网易邮箱手机版
在2017年11月27日 23:42，Vladimir Marek 写道：
> > It's not the point. the problem is bash-4.4 auto add quote around $var or
> > $(cmd) after '<<<'
> >
> > On bash-4.2 and before following command always works fine
> > read ignore fstype <<<$(df --output=fstype $mountpoint)
> >
> > But after Update bash-4.4, our customer's script get fail, do you
> > understand what I mean?
>
> The script is broken. It happened to work in bash 4.2 because of a bug
> in bash 4.2 that cancelled out the bug in the script.
>
> wooledg:~$ df --output=fstype /
> Type
> ext4
>
> Here is how the script should be written:
>
> { read; read -r fstype; } < <(df --output=fstype "$mountpoint")
>
> And testing it:
>
> wooledg:~$ mountpoint=/
> wooledg:~$ { read; read -r fstype; } < <(df --output=fstype "$mountpoint")
> wooledg:~$ declare -p fstype
> declare -- fstype="ext4"
Or even
fstype=$( stat --file-system --print "%T\n" "$mountpoint" )
--
Vlad
Re: [PATCH] Fix hang if $OLDPWD points to inaccessible directory
On Mon, Nov 27, 2017 at 6:00 PM, Mikulas Patocka wrote: [...] > Will you commit my patch? - if you agree with it. Hi Mikulas. Chet is the one and only bash maintainer, you'd have to convince him. As a bystander that happens to be interested in bash's development, I agree with your points, but expressing my opinion is the most I can do.
Re: Bash: sleep execution issue with bash loadable builtins
On 11/28/17 12:17 AM, Thiruvadi Rajaraman wrote: > Hi, > > Thanks a lot for your review comments. > > I have reworked on the bash sleep fix based on your suggestion about signal > and trap handling in fsleep( ). > > I have attached the fix patch for your kind reference. Your patch unconditionally changes the SIGCHLD signal handler to an invalid value (you probably meant to use SIG_IGN) without restoring it. An interactive shell would not be able to use job control until something internal reset the SIGCHLD handler to the correct value. An approach that uses pselect() if available and blocks SIGCHLD for the duration of the call, as Angel suggested, is probably the best approach. Chet -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/
Re: Bash: sleep execution issue with bash loadable builtins
On 11/27/17 2:47 PM, Ángel wrote: > Also there's the issue that select() _may_ modify the object pointed to > by the timeout argument [POSIX]. But it may not, in which case this > would end up oversleeping. > > On such system, doing eg. > sleep 9 & time sleep 10 > > would end up sleeping 19 seconds. I can't see how, since the two sleeps would be performed by different processes, using two separately-initialized select calls. -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/
Re: Bash: sleep execution issue with bash loadable builtins
On 11/28/17 9:41 AM, Chet Ramey wrote: > On 11/27/17 2:47 PM, Ángel wrote: > >> Also there's the issue that select() _may_ modify the object pointed to >> by the timeout argument [POSIX]. But it may not, in which case this >> would end up oversleeping. >> >> On such system, doing eg. >> sleep 9 & time sleep 10 >> >> would end up sleeping 19 seconds. > > I can't see how, since the two sleeps would be performed by different > processes, using two separately-initialized select calls. Forget this, I see it. -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/
Re: [PATCH] Fix hang if $OLDPWD points to inaccessible directory
On 11/27/17 4:00 PM, Mikulas Patocka wrote: > Will you commit my patch? - if you agree with it. I believe it's a good idea to restrict importing values of OLDPWD to directories, but I will provide a configuration option (config-top.h for now) to optionally disable it. Chet -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/
