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:
parent
c3388ffd85
commit
ff385e5a89
4 changed files with 40 additions and 232 deletions
5
NEWS
5
NEWS
|
@ -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)
|
||||
|
|
|
@ -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"
|
||||
|
|
|
@ -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))
|
||||
|
|
|
@ -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)
|
||||
|
|
Loading…
Reference in a new issue