1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-02-15 04:32:24 +00:00

Fix "/dev/null: cannot create [Bad file descriptor]" (re: feeb62d1)

Intermittent regression test failure:

test coprocess begins at 2022-02-08+23:41:52
/usr/local/src/ksh93/ksh/src/cmd/ksh93/tests/coprocess.sh[116]: /dev/null: cannot create [Bad file descriptor]
	coprocess.sh[118]: FAIL: /bin/cat coprocess hung after 'exec 5<&p 6>&p; exec 5<&- 6>&-'
test coprocess failed at 2022-02-08+23:41:56 with exit code 1 [ 33 tests 1 error ]

The coprocess didn't hang; the 2>/dev/null redirection in the
regression test somehow failed.

Running the regression test in a loop 10000 times is a fairly
reliable way of reproducing the failure on my system. This has
allowed me to find the commit that introduced it. It is: feeb62d1

How odd. That commit merely sets errno to EBADF if sh_close() is
called with a file descriptor < 0. I don't understand how this
causes a race condition.

But calling abort() in the location of the feeb62d1 patch did allow
me to trace all the sh_close() calls with a negative file
descriptor. Those shouldn't be happening in any case.

All those sh_close() calls were related to co-processes, which that
regression test also tests. A co-process pipe that is not in use is
set to -1 so we need to avoid calling sh_close() if that happened.

This commit does that (as well as making some consistency tweaks)
and it reliably makes the race condition go away on my system. The
"why" of this remains a mystery as the only difference is that
errno is not set to EBADF in these situations, and both
sh_redirect() and sh_open() explicitly set errno to 0.

Resolves: https://github.com/ksh93/ksh/issues/483
This commit is contained in:
Martijn Dekker 2022-06-20 03:16:57 +01:00
parent 627058e862
commit a72ba2cf59
4 changed files with 16 additions and 10 deletions

View file

@ -1045,9 +1045,9 @@ static Sfoff_t file_offset(int fn, char *fname)
*/
void sh_pclose(register int pv[])
{
if(pv[0]>=2)
if(pv[0] > -1)
sh_close(pv[0]);
if(pv[1]>=2)
if(pv[1] > -1)
sh_close(pv[1]);
pv[0] = pv[1] = -1;
}

View file

@ -396,10 +396,11 @@ int job_reap(register int sig)
/* check for coprocess completion */
if(pid==sh.cpid)
{
sh_close(sh.coutpipe);
sh_close(sh.cpipe[1]);
sh.cpipe[1] = -1;
sh.coutpipe = -1;
if(sh.coutpipe > -1)
sh_close(sh.coutpipe);
if(sh.cpipe[1] > -1)
sh_close(sh.cpipe[1]);
sh.coutpipe = sh.cpipe[1] = -1;
}
else if(sh.subshell)
sh_subjobcheck(pid);

View file

@ -1275,10 +1275,13 @@ static noreturn void exscript(register char *path,register char *argv[],char **e
sh.bckpid = 0;
sh.st.ioset=0;
/* clean up any cooperating processes */
if(sh.cpipe[0]>0)
if(sh.cpipe[0] > -1)
sh_pclose(sh.cpipe);
if(sh.cpid && sh.outpipe)
if(sh.cpid && sh.outpipe && *sh.outpipe > -1)
{
sh_close(*sh.outpipe);
*sh.outpipe = -1;
}
sh.cpid = 0;
if(sp=fcfile())
while(sfstack(sp,SF_POPSTACK));

View file

@ -455,8 +455,10 @@ void sh_subjobcheck(pid_t pid)
{
if(sp->cpid==pid)
{
sh_close(sp->coutpipe);
sh_close(sp->cpipe);
if(sp->coutpipe > -1)
sh_close(sp->coutpipe);
if(sp->cpipe > -1)
sh_close(sp->cpipe);
sp->coutpipe = sp->cpipe = -1;
return;
}