Re: Bash: sleep execution issue with bash loadable builtins

[Thiruvadi Rajaraman]

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]+  Done                    sleep 1
[1] 19524
Tue Nov 28 07:18:03 UTC 2017
Tue Nov 28 07:18:13 UTC 2017
[1]+  Done                    sleep 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 <chet.ra...@case.edu> 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, CWRU    c...@case.edu    http://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 <trajara...@mvista.com>
 
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 <signal.h>
 #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;