Skip to content

Commit

Permalink
Fix three bugs in the sleep builtin
Browse files Browse the repository at this point in the history
This commit backports the main changes to sh_delay from ksh93v-
and ksh2020, which fixes the following bugs:
- Microsecond amounts of less than one millisecond are no longer
  ignored. The following loop will now take a minimum of one
  second to complete:
  for ((i = 0; i != 10000; i++)) do
    sleep PT100U
  done

- 'sleep 30' no longer adds an extra 30 milliseconds to the total
  amount of time to sleep. This bug is hard to notice since 30
  milliseconds can be considered within the margin of error. The
  only reason why longer delays weren't affected is because the old
  code masked the bug when the interval is greater than 30 seconds:
  else if(n > 30)
  {
      sleep(n);
      t -= n;
  }
  This caused 'sleep -s' to break with intervals greater than 30
  seconds, so an actual fix is used instead of a workaround.

- 'sleep -s' now functions correctly with intervals of more than
  30 seconds as the new code doesn't need the old workaround. This
  is done by handling '-s' in sh_delay.

src/cmd/ksh93/bltins/sleep.c:
- Remove the replacement for sleep(3) from the sleep builtin.
- Replace the old sh_delay function with the newer one from ksh2020.
  The new function uses tvsleep, which uses nanosleep(3) internally.

src/cmd/ksh93/include/shell.h,
src/cmd/ksh93/edit/edit.c,
src/cmd/ksh93/sh/jobs.c,
src/cmd/ksh93/sh/xec.c,
src/cmd/ksh93/shell.3:
- Update sh_delay documentation and usage since the function now
  requires two arguments.

src/cmd/ksh93/tests/builtins.sh:
- Add a regression test for 'sleep -s' when the interval is greater
  than 30 seconds. The other bugs can't be tested for in a feasible
  manner across all systems:
  ksh93#72 (comment)
  • Loading branch information
JohnoKing committed Jul 16, 2020
1 parent 17f81eb commit e2d3d7f
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 108 deletions.
9 changes: 9 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,15 @@ For full details, see the git log at: https://github.com/ksh93/ksh

Any uppercase BUG_* names are modernish shell bug IDs.

2020-07-16:

- Fixed a bug that caused 'sleep -s' to have no effect with intervals longer
than 30 seconds.

- The accuracy of the sleep builtin has been improved. It no longer ignores
microseconds and doesn't add extra milliseconds when the interval is less
than 31 seconds.

2020-07-15:

- The 'autoload', 'compound', 'float', 'functions', 'integer' and 'nameref'
Expand Down
113 changes: 12 additions & 101 deletions src/cmd/ksh93/bltins/sleep.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@
*
*/

#define sleep ______sleep
#include "defs.h"
#undef sleep
#include <error.h>
#include <errno.h>
#include <tmx.h>
Expand Down Expand Up @@ -114,7 +112,7 @@ int b_sleep(register int argc,char *argv[],Shbltin_t *context)
time_t now;
errno = 0;
shp->lastsig=0;
sh_delay(d);
sh_delay(d,sflag);
if(sflag || tloc==0 || errno!=EINTR || shp->lastsig)
break;
sh_sigcheck(shp);
Expand All @@ -127,109 +125,22 @@ int b_sleep(register int argc,char *argv[],Shbltin_t *context)
return(0);
}

static void completed(void * handle)
{
char *expired = (char*)handle;
*expired = 1;
}

unsigned int sleep(unsigned int sec)
{
Shell_t *shp = sh_getinterp();
pid_t newpid, curpid=getpid();
void *tp;
char expired = 0;
shp->lastsig = 0;
tp = (void*)sh_timeradd(1000*sec, 0, completed, (void*)&expired);
do
{
if(!shp->gd->waitevent || (*shp->gd->waitevent)(-1,-1L,0)==0)
pause();
if(shp->sigflag[SIGALRM]&SH_SIGTRAP)
sh_timetraps(shp);
if((newpid=getpid()) != curpid)
{
curpid = newpid;
shp->lastsig = 0;
shp->trapnote &= ~SH_SIGSET;
if(expired)
expired = 0;
else
timerdel(tp);
tp = (void*)sh_timeradd(1000*sec, 0, completed, (void*)&expired);
}
}
while(!expired && shp->lastsig==0);
if(!expired)
timerdel(tp);
sh_sigcheck(shp);
return(0);
}

/*
* delay execution for time <t>
*/

void sh_delay(double t)
void sh_delay(double t, int sflag)
{
register int n = (int)t;
Shell_t *shp = sh_getinterp();
#ifdef _lib_poll
struct pollfd fd;
if(t<=0)
return;
else if(n > 30)
{
sleep(n);
t -= n;
}
if(n=(int)(1000*t))
Shell_t *shp = sh_getinterp();
int n = (int)t;
Tv_t ts, tx;

ts.tv_sec = n;
ts.tv_nsec = 1000000000 * (t - (double)n);
while(tvsleep(&ts, &tx) < 0 && errno == EINTR)
{
if(!shp->gd->waitevent || (*shp->gd->waitevent)(-1,(long)n,0)==0)
poll(&fd,0,n);
}
#else
# if defined(_lib_select) && defined(_mem_tv_usec_timeval)
struct timeval timeloc;
if(t<=0)
return;
if(n=(int)(1000*t) && shp->gd->waitevent && (*shp->gd->waitevent)(-1,(long)n,0))
return;
n = (int)t;
timeloc.tv_sec = n;
timeloc.tv_usec = 1000000*(t-(double)n);
select(0,(fd_set*)0,(fd_set*)0,(fd_set*)0,&timeloc);
# else
# ifdef _lib_select
/* for 9th edition machines */
if(t<=0)
return;
if(n > 30)
{
sleep(n);
t -= n;
}
if(n=(int)(1000*t))
{
if(!shp->gd->waitevent || (*shp->gd->waitevent)(-1,(long)n,0)==0)
select(0,(fd_set*)0,(fd_set*)0,n);
}
# else
struct tms tt;
if(t<=0)
if ((shp->trapnote & (SH_SIGSET | SH_SIGTRAP)) || sflag)
return;
sleep(n);
t -= n;
if(t)
{
clock_t begin = times(&tt);
if(begin==0)
return;
t *= shp->gd->lim.clk_tck;
n += (t+.5);
while((times(&tt)-begin) < n);
}
# endif
# endif
#endif /* _lib_poll */
ts = tx;
}
}
2 changes: 1 addition & 1 deletion src/cmd/ksh93/edit/edit.c
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,7 @@ int ed_read(void *context, int fd, char *buff, int size, int reedit)
}
ep->sh->winch = 0;
ed_flush(ep);
sh_delay(.05);
sh_delay(.05,0);
astwinsize(2,&rows,&newsize);
ep->e_winsz = newsize-1;
if(ep->e_winsz < MINWINDOW)
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/shell.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ extern void sh_subfork(void);
extern Shell_t *sh_init(int,char*[],Shinit_f);
extern int sh_reinit(char*[]);
extern int sh_eval(Sfio_t*,int);
extern void sh_delay(double);
extern void sh_delay(double,int);
extern void *sh_parse(Shell_t*, Sfio_t*,int);
extern int sh_trap(const char*,int);
extern int sh_fun(Namval_t*,Namval_t*, char*[]);
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/ksh93/sh/jobs.c
Original file line number Diff line number Diff line change
Expand Up @@ -1188,7 +1188,7 @@ int job_kill(register struct process *pw,register int sig)
job_unstop(pw);
#endif /* SIGTSTP */
if(r>=0)
sh_delay(.05);
sh_delay(.05,0);
}
while(pw && pw->p_pgrp==0 && (r=kill(pw->p_pid,sig))>=0)
{
Expand All @@ -1211,7 +1211,7 @@ int job_kill(register struct process *pw,register int sig)
sfprintf(sfstderr,"kill: %s: %s\n",job_string, msg);
r = 2;
}
sh_delay(.001);
sh_delay(.001,0);
job_unlock();
return(r);
}
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/ksh93/sh/xec.c
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ static void iounpipe(Shell_t *shp)
job.in_critical--;
break;
}
sh_delay(1);
sh_delay(1,0);
}
if((n = read(subpipe[0],buff,sizeof(buff)))==0)
break;
Expand Down Expand Up @@ -1935,7 +1935,7 @@ int sh_exec(register const Shnode_t *t, int flags)
timerdel(fifo_timer);
sh_iorenumber(shp,fn,fd);
sh_close(fn);
sh_delay(.001);
sh_delay(.001,0);
unlink(shp->fifo);
free(shp->fifo);
shp->fifo = 0;
Expand Down
5 changes: 4 additions & 1 deletion src/cmd/ksh93/shell.3
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ Shscope_t *sh_setscope(Shscope_t *\fIscope\fP);
int (*sh_fdnotify(int(*\fIfn\fP)(int,int)))(int,int);
char *sh_fmtq(const char *\fIstring\fP);
void *sh_waitnotify(Shwait_f \fIfn\fP);
void sh_delay(double \fIsec\fP);
void sh_delay(double \fIsec\fP, int \fIsflag\fP);
Sfio_t *sh_iogetiop(int \fIfd\fP, int \fImode\fP);
int sh_sigcheck(void);
.ft R
Expand Down Expand Up @@ -349,6 +349,9 @@ where the most recent built-in was invoked, or where
.PP
The \f5sh_delay()\fP function is used to cause the
shell to sleep for a period of time defined by \fIsec\fP.
If \fIsflag\fP is true, the shell will stop sleeping when
any signal is received; otherwise signals such as \f5SIGCONT\fP
and \f5SIGINFO\fP are treated normally.
.PP
The \f5sh_fmtq()\fP function can be used to convert a string
into a string that is quoted so that it can be reinput
Expand Down
16 changes: 16 additions & 0 deletions src/cmd/ksh93/tests/builtins.sh
Original file line number Diff line number Diff line change
Expand Up @@ -846,5 +846,21 @@ IMPLEMENTATION
[[ $actual == "$expect" ]] || err_exit "getopts: '--man' output" \
"(expected $(printf %q "$expect"), got $(printf %q "$actual"))"
# ======
# 'sleep -s' should work in interactive shells when seconds > 30.
sleepsig="$tmp/sleepsig.sh"
cat >| "$sleepsig" << 'EOF'
sleep -s 31 &
sleep .001
kill -CONT $!
if kill -0 $!; then
kill -TERM $! # Don't leave a lingering background process
exit 1
else
exit 0
fi
EOF
"$SHELL" -i "$sleepsig" 2> /dev/null || err_exit "'sleep -s' doesn't work with intervals of more than 30 seconds"
# ======
exit $((Errors<125?Errors:125))

0 comments on commit e2d3d7f

Please sign in to comment.