1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-02-13 11:42:21 +00:00

Speed up 'read', fixing macOS hang

This fixes a hanging bug that could occur on macOS when using the
'read' command to read from a FIFO and encountering end-of-file
without a final newline character. It also makes the 'read' command
perform 15-25% faster on macOS and Linux and maybe other systems.

src/lib/libast/sfio/sfpkrd.c: sfpkrd():
- Get rid of the optional stuff that uses the poll(2) or select(2)
  syscalls. The only thing that is required to avoid regressions is
  the code that was conditional upon the socket_peek feature test,
  which tests for the correct functioning of the recv(2) syscall.
  This has now been made mandatory. The rest now uses what was
  previously a fallback in plain C, resulting in a function that is
  not only more readable, but actually faster than the syscalls.

Resolves: https://github.com/ksh93/ksh/issues/118
This commit is contained in:
Martijn Dekker 2020-08-19 01:36:01 +01:00
parent c3388ffd85
commit ff385e5a89
4 changed files with 40 additions and 232 deletions

5
NEWS
View file

@ -3,6 +3,11 @@ For full details, see the git log at: https://github.com/ksh93/ksh
Any uppercase BUG_* names are modernish shell bug IDs.
2020-08-18:
- Sped up the 'read' command on most systems by 15-25%. Fixed a hanging bug
on reading from a FIFO that could occur on macOS.
2020-08-17:
- 'command -p' incorrectly used the hash table entry (a.k.a. tracked alias)

View file

@ -17,4 +17,4 @@
* David Korn <dgk@research.att.com> *
* *
***********************************************************************/
#define SH_RELEASE "93u+m 2020-08-17"
#define SH_RELEASE "93u+m 2020-08-18"

View file

@ -583,5 +583,19 @@ then exec 3>&-
err_exit "Open file descriptor leaks out of subshell"
fi
# ======
# On unpatched ksh on macOS, 'read' used to block when reading from a FIFO and there was no final newline.
if mkfifo "$tmp/fifo_no_lf"
then trap 'sleep_pid=0; kill "$ksh_pid"; err_exit "'\''read'\'' hangs on EOF without final linefeed when reading from FIFO"' TERM
(sleep 1; kill "$$") &
sleep_pid=$!
"$SHELL" -c 'print -n foo >$0 & while read f; do :; done <$0' "$tmp/fifo_no_lf" &
ksh_pid=$!
wait "$ksh_pid"
trap - TERM
((sleep_pid)) && kill "$sleep_pid"
else err_exit "mkfifo failed; cannot test reading from FIFO"
fi
# ======
exit $((Errors<125?Errors:125))

View file

@ -20,11 +20,12 @@
* *
***********************************************************************/
#include "sfhdr.h"
#if !_PACKAGE_ast
#ifndef FIONREAD
#if _sys_ioctl
#include <sys/ioctl.h>
#endif
#if !_socket_peek
#if __APPLE__
#error The socket_peek feature is required. (Hey Apple, revert your src__lib__libast__features__lib.diff patch; it caused multiple regressions, and the hanging bug it fixed is now fixed correctly. See <https://github.com/ksh93/ksh/issues/118>.)
#else
#error The socket_peek feature is required.
#endif
#endif
@ -33,9 +34,6 @@
** Written by Kiem-Phong Vo.
*/
#define STREAM_PEEK 001
#define SOCKET_PEEK 002
#if __STD_C
ssize_t sfpkrd(int fd, Void_t* argbuf, size_t n, int rc, long tm, int action)
#else
@ -52,237 +50,28 @@ int action; /* >0: peeking, if rc>=0, get action records,
#endif
{
reg ssize_t r;
reg int ntry, t;
reg int t;
reg char *buf = (char*)argbuf, *endbuf;
if(rc < 0 && tm < 0 && action <= 0)
return sysreadf(fd,buf,n);
t = (action > 0 || rc >= 0) ? (STREAM_PEEK|SOCKET_PEEK) : 0;
#if !_stream_peek
t &= ~STREAM_PEEK;
#endif
#if !_socket_peek
t &= ~SOCKET_PEEK;
#endif
for(ntry = 0; ntry < 2; ++ntry)
{
r = -1;
#if _stream_peek
if((t&STREAM_PEEK) && (ntry == 1 || tm < 0) )
{
#ifdef __sun
/*
* I_PEEK on stdin can hang rsh+ksh on solaris
* this kludge will have to do until sun^H^H^Horacle fixes I_PEEK/rsh
*/
static int stream_peek;
if (stream_peek == 0) /* this will be done just once */
{ char *e;
stream_peek = (
getenv("LOGNAME") == 0 &&
getenv("MAIL") == 0 &&
((e = getenv("LANG")) == 0 || strcmp(e, "C") == 0) &&
((e = getenv("PATH")) == 0 || strncmp(e, "/usr/bin:", 9) == 0)
) ? -1 : 1;
}
if(stream_peek < 0)
t &= ~STREAM_PEEK;
else
#endif
{ struct strpeek pbuf;
pbuf.flags = 0;
pbuf.ctlbuf.maxlen = -1;
pbuf.ctlbuf.len = 0;
pbuf.ctlbuf.buf = NIL(char*);
pbuf.databuf.maxlen = n;
pbuf.databuf.buf = buf;
pbuf.databuf.len = 0;
if((r = ioctl(fd,I_PEEK,&pbuf)) < 0)
{ if(errno == EINTR)
return -1;
t &= ~STREAM_PEEK;
}
else
{ t &= ~SOCKET_PEEK;
if(r > 0 && (r = pbuf.databuf.len) <= 0)
{ if(action <= 0) /* read past eof */
r = sysreadf(fd,buf,1);
return r;
}
if(r == 0)
r = -1;
else if(r > 0)
break;
}
}
}
#endif /* stream_peek */
if(ntry == 1)
break;
/* poll or select to see if data is present. */
while(tm >= 0 || action > 0 ||
/* block until there is data before peeking again */
((t&STREAM_PEEK) && rc >= 0) ||
/* let select be interrupted instead of recv which autoresumes */
(t&SOCKET_PEEK) )
{ r = -2;
#if _lib_poll
if(r == -2)
{
struct pollfd po;
po.fd = fd;
po.events = POLLIN;
po.revents = 0;
if((r = SFPOLL(&po,1,tm)) < 0)
{ if(errno == EINTR)
return -1;
else if(errno == EAGAIN)
{ errno = 0;
continue;
}
else r = -2;
}
else r = (po.revents&POLLIN) ? 1 : -1;
}
#endif /*_lib_poll*/
#if _lib_select
if(r == -2)
{
#if _hpux_threads && vt_threaded
#define fd_set int
#endif
fd_set rd;
struct timeval tmb, *tmp;
FD_ZERO(&rd);
FD_SET(fd,&rd);
if(tm < 0)
tmp = NIL(struct timeval*);
else
{ tmp = &tmb;
tmb.tv_sec = tm/SECOND;
tmb.tv_usec = (tm%SECOND)*SECOND;
}
r = select(fd+1,&rd,NIL(fd_set*),NIL(fd_set*),tmp);
if(r < 0)
{ if(errno == EINTR)
return -1;
else if(errno == EAGAIN)
{ errno = 0;
continue;
}
else r = -2;
}
else r = FD_ISSET(fd,&rd) ? 1 : -1;
}
#endif /*_lib_select*/
if(r == -2)
{
#if !_lib_poll && !_lib_select /* both poll and select can't be used */
#ifdef FIONREAD /* quick and dirty check for availability */
long nsec = tm < 0 ? 0 : (tm+999)/1000;
while(nsec > 0 && r < 0)
{ long avail = -1;
if((r = ioctl(fd,FIONREAD,&avail)) < 0)
{ if(errno == EINTR)
return -1;
else if(errno == EAGAIN)
{ errno = 0;
continue;
}
else /* ioctl failed completely */
{ r = -2;
break;
}
}
else r = avail <= 0 ? -1 : (ssize_t)avail;
if(r < 0 && nsec-- > 0)
sleep(1);
}
#endif
#endif
}
if(r > 0) /* there is data now */
{ if(action <= 0 && rc < 0)
return sysreadf(fd,buf,n);
else r = -1;
}
else if(tm >= 0) /* timeout exceeded */
r = -1;
t = action > 0 || rc >= 0; /* socket peek using recv(2)? */
if(t)
{ while(t && (r = recv(fd,(char*)buf,n,MSG_PEEK)) < 0)
{ if(errno == EINTR)
return -1;
else r = -1;
break;
else if(errno == EAGAIN)
errno = 0;
else t = 0;
}
#if _socket_peek
if(t&SOCKET_PEEK)
{
#if __MACH__ && __APPLE__ /* check 10.4 recv(MSG_PEEK) bug that consumes pipe data */
static int recv_peek_pipe;
if (recv_peek_pipe == 0) /* this will be done just once */
{ int fds[2], r;
char tst[2];
tst[0] = 'a'; tst[1] = 'z';
/* open a pipe and write to it */
recv_peek_pipe = 1;
if(recv_peek_pipe == 1 && pipe(fds) < 0)
recv_peek_pipe = -1;
if(recv_peek_pipe == 1 && write(fds[1], tst, 2) != 2)
recv_peek_pipe = -1;
/* try recv() to see if it gets anything */
tst[0] = tst[1] = 0;
if(recv_peek_pipe == 1 && (r = recv(fds[0], tst, 1, MSG_PEEK)) != 1)
recv_peek_pipe = -1;
if(recv_peek_pipe == 1 && tst[0] != 'a')
recv_peek_pipe = -1;
/* make sure that recv() did not consume data */
tst[0] = tst[1] = 0;
if(recv_peek_pipe == 1 && (r = recv(fds[0], tst, 2, MSG_PEEK)) != 2)
recv_peek_pipe = -1;
if(recv_peek_pipe == 1 && (tst[0] != 'a' || tst[1] != 'z') )
recv_peek_pipe = -1;
close(fds[0]);
close(fds[1]);
}
if(recv_peek_pipe < 0)
{ struct stat st; /* recv should work on sockets */
if(fstat(fd, &st) < 0 || !S_ISSOCK(st.st_mode) )
{ r = -1;
t &= ~SOCKET_PEEK;
}
}
#endif
while((t&SOCKET_PEEK) && (r = recv(fd,(char*)buf,n,MSG_PEEK)) < 0)
{ if(errno == EINTR)
return -1;
else if(errno == EAGAIN)
errno = 0;
else t &= ~SOCKET_PEEK;
}
if(r >= 0)
{ t &= ~STREAM_PEEK;
if(r > 0)
break;
else /* read past eof */
{ if(action <= 0)
r = sysreadf(fd,buf,1);
return r;
}
}
if(r == 0)
{ /* read past eof */
if(action <= 0)
r = sysreadf(fd,buf,1);
return r;
}
#endif
}
if(r < 0)