zsh-workers
 help / color / mirror / code / Atom feed
* Zsh 4.3.12: subshell in midnight commander:  precmd: 15: bad file descriptor
@ 2011-07-18  8:26 Pavel Reznicek
  2011-07-18  9:07 ` Peter Stephenson
  2011-07-18 14:13 ` Peter Stephenson
  0 siblings, 2 replies; 14+ messages in thread
From: Pavel Reznicek @ 2011-07-18  8:26 UTC (permalink / raw)
  To: zsh-workers


Hello,

   after upgrading to zsh 4.3.12 (in debian), I am having problems running 
midnight commander in zsh (going back to zsh version 4.3.10 it works 
fine). The symptom is that when trying to use the subshell in mc, it 
complains about:

precmd: 15: bad file descriptor

and the mc session get hanged. Of course it could be bug in mc and not 
zsh, but since this behaviour is not seen with zsh 4.3.10, the zsh updates 
since that version can tell what is going on. Browsing the mc source code, 
the precmd command defined in there is:

precmd(){ pwd>&%d;kill -STOP $$ }

Would some have an idea why this precmd command would cause the error 
message above in connection of the updateds between zsh 4.3.10 and 4.3.12 
?

PS: It is also important to note that the behaviour seem to be
     terminal-emulator dependent (happens in konsole and urxvt, but not in
     gnome-terminal, aterm).

Thanks,
Pavel


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Zsh 4.3.12: subshell in midnight commander:  precmd: 15: bad file descriptor
  2011-07-18  8:26 Zsh 4.3.12: subshell in midnight commander: precmd: 15: bad file descriptor Pavel Reznicek
@ 2011-07-18  9:07 ` Peter Stephenson
  2011-07-18 10:04   ` Pavel Reznicek
  2011-07-18 14:13 ` Peter Stephenson
  1 sibling, 1 reply; 14+ messages in thread
From: Peter Stephenson @ 2011-07-18  9:07 UTC (permalink / raw)
  To: Pavel Reznicek, zsh-workers

On Mon, 18 Jul 2011 10:26:27 +0200
Pavel Reznicek <reznicek@ipnp.troja.mff.cuni.cz> wrote:
>    after upgrading to zsh 4.3.12 (in debian), I am having problems running 
> midnight commander in zsh (going back to zsh version 4.3.10 it works 
> fine). The symptom is that when trying to use the subshell in mc, it 
> complains about:
> 
> precmd: 15: bad file descriptor
> 
> and the mc session get hanged. Of course it could be bug in mc and not 
> zsh, but since this behaviour is not seen with zsh 4.3.10, the zsh updates 
> since that version can tell what is going on. Browsing the mc source code, 
> the precmd command defined in there is:
> 
> precmd(){ pwd>&%d;kill -STOP $$ }
> 
> Would some have an idea why this precmd command would cause the error 
> message above in connection of the updateds between zsh 4.3.10 and 4.3.12 
> ?

This could well be associated with a job control change which I hope was
fixed by Bart in zsh-workers/29481
(http://www.zsh.org/mla/workers/2011/msg00915.html).

If it does fix it, it might well be worth releasing a 4.3.13 with this
in.

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Zsh 4.3.12: subshell in midnight commander:  precmd: 15: bad file descriptor
  2011-07-18  9:07 ` Peter Stephenson
@ 2011-07-18 10:04   ` Pavel Reznicek
  2011-07-18 14:07     ` Bart Schaefer
  2011-07-18 14:45     ` Bart Schaefer
  0 siblings, 2 replies; 14+ messages in thread
From: Pavel Reznicek @ 2011-07-18 10:04 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: zsh-workers


>>    after upgrading to zsh 4.3.12 (in debian), I am having problems running
>> midnight commander in zsh (going back to zsh version 4.3.10 it works
>> fine). The symptom is that when trying to use the subshell in mc, it
>> complains about:
>>
>> precmd: 15: bad file descriptor
>>
>> and the mc session get hanged. Of course it could be bug in mc and not
>> zsh, but since this behaviour is not seen with zsh 4.3.10, the zsh updates
>> since that version can tell what is going on. Browsing the mc source code,
>> the precmd command defined in there is:
>>
>> precmd(){ pwd>&%d;kill -STOP $$ }
>>
>> Would some have an idea why this precmd command would cause the error
>> message above in connection of the updateds between zsh 4.3.10 and 4.3.12
>> ?
>
> This could well be associated with a job control change which I hope was
> fixed by Bart in zsh-workers/29481
> (http://www.zsh.org/mla/workers/2011/msg00915.html).


Unfortunatelly, it does not fix it (included the patch in the thread on
top of debian zsh 4.3.12-1).

Pavel


> If it does fix it, it might well be worth releasing a 4.3.13 with this
> in.
>
> -- 
> Peter Stephenson <pws@csr.com>            Software Engineer
> Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
> Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK
>
>
> Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
> More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog
>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Zsh 4.3.12: subshell in midnight commander:  precmd: 15: bad file descriptor
  2011-07-18 10:04   ` Pavel Reznicek
@ 2011-07-18 14:07     ` Bart Schaefer
  2011-07-18 14:45     ` Bart Schaefer
  1 sibling, 0 replies; 14+ messages in thread
From: Bart Schaefer @ 2011-07-18 14:07 UTC (permalink / raw)
  To: zsh-workers

On Jul 18, 12:04pm, Pavel Reznicek wrote:
}
} >>    after upgrading to zsh 4.3.12 (in debian), I am having problems running
} >> midnight commander in zsh (going back to zsh version 4.3.10 it works
} >> fine). The symptom is that when trying to use the subshell in mc, it
} >> complains about:
} >>
} >> precmd: 15: bad file descriptor
} >>
} >> precmd(){ pwd>&%d;kill -STOP $$ }
} >
} > This could well be associated with a job control change which I hope was
} > fixed by Bart in zsh-workers/29481
} > (http://www.zsh.org/mla/workers/2011/msg00915.html).
} 
} Unfortunatelly, it does not fix it (included the patch in the thread on
} top of debian zsh 4.3.12-1).

Other changes which might be related:

      * 29368: Src/exec.c: do not restore xtrerr to stderr before
      running simple commands; restore xtrerr to stderr just before
      running a function body, but after printing the trace of
      the function call itself.

      * 29129: Src/exec.c: reading off end of file descriptor array

      * 28762: Src/exec.c: logic for closing coproc file descriptors
      was wrong.

      * 28461: Src/exec.c: flush stderr in PRINT_EXIT_VALUE handling.

      * 27721: Src/compat.c [with unnecessary test removed], Src/exec.c,
      Src/system.h, Src/utils.c:  update zopenmax() not to examine huge
      numbers of file descriptors; only call it at initialisation;
      rationalise use of fdtable_size and expansion of fdtable.

      * 27284: Src/exec.c, Src/parse.c, Src/utils.c,
      Src/Modules/socket.c, Src/Modules/tcp.c, Src/Modules/zpty.c:
      improve use of movefd() and restore closing of original fd
      on failure pending further work.

      * 27283: Src/exec.c, Src/utils.c: failure to dup fd accessed
      uninitialised memory and closed the original fd regardless.

      * 27134: Src/exec.c, Src/init.c: improve 27131 by setting
      FD_CLOEXEC for SHTTY or closing it by hand when executing.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Zsh 4.3.12: subshell in midnight commander:  precmd: 15: bad file descriptor
  2011-07-18  8:26 Zsh 4.3.12: subshell in midnight commander: precmd: 15: bad file descriptor Pavel Reznicek
  2011-07-18  9:07 ` Peter Stephenson
@ 2011-07-18 14:13 ` Peter Stephenson
  2011-10-02 19:00   ` Pavel Reznicek
  1 sibling, 1 reply; 14+ messages in thread
From: Peter Stephenson @ 2011-07-18 14:13 UTC (permalink / raw)
  To: Pavel Reznicek, zsh-workers

On Mon, 18 Jul 2011 10:26:27 +0200
Pavel Reznicek <reznicek@ipnp.troja.mff.cuni.cz> wrote:
>    after upgrading to zsh 4.3.12 (in debian), I am having problems running 
> midnight commander in zsh (going back to zsh version 4.3.10 it works 
> fine). The symptom is that when trying to use the subshell in mc, it 
> complains about:
> 
> precmd: 15: bad file descriptor
> 
> and the mc session get hanged. Of course it could be bug in mc and not 
> zsh, but since this behaviour is not seen with zsh 4.3.10, the zsh updates 
> since that version can tell what is going on. Browsing the mc source code, 
> the precmd command defined in there is:
> 
> precmd(){ pwd>&%d;kill -STOP $$ }
> 
> Would some have an idea why this precmd command would cause the error 
> message above in connection of the updateds between zsh 4.3.10 and 4.3.12 
> ?

So if it's not (obviously) related to the "stop", we probably need to
know what file descriptor is appearing there.  If it's 1 or 2 there's
not obvious reason for parametrising it.  If it's not, it's not obvious
what it would be --- something opened by mc, presumably, and possibly
getting FD_CLOEXEC set, if the problem only occurs in subshells.

Hmm... if it were the shell terminal output, this fix spotted by Bart
might be relevant:

    * 27134: Src/exec.c, Src/init.c: improve 27131 by setting
      FD_CLOEXEC for SHTTY or closing it by hand when executing.

but SHTTY isn't directly exposed to the shell user (which is why
we're closing it).

pws


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Zsh 4.3.12: subshell in midnight commander:  precmd: 15: bad file descriptor
  2011-07-18 10:04   ` Pavel Reznicek
  2011-07-18 14:07     ` Bart Schaefer
@ 2011-07-18 14:45     ` Bart Schaefer
  2011-07-18 15:24       ` Peter Stephenson
  1 sibling, 1 reply; 14+ messages in thread
From: Bart Schaefer @ 2011-07-18 14:45 UTC (permalink / raw)
  To: zsh-workers

On Jul 18, 12:04pm, Pavel Reznicek wrote:
}
} >> precmd: 15: bad file descriptor
} >>
} >> the precmd command defined in there is:
} >>
} >> precmd(){ pwd>&%d;kill -STOP $$ }

Hmm.  I was thinking that the error had to be coming from somewhere only
indirectly related to this because >&15 would be a parse error, but in
fact zsh accepts multi-digit descriptor numbers on the right side of >&
(but not on the left side, which bash does and ksh does not).  So MC is
explicitly sprintf'ing a descriptor that it created into that pwd.

Thus this may be revealing:

torch% exec {stderr}>&2
torch% print $stderr
11
torch% Src/zsh -f
torch% precmd() { pwd>&11 }     
precmd: 11: bad file descriptor
torch% 

Now here's 4.3.9 (I don't have .10 handy):

macadamia% exec {stderr}>&2
macadamia% print $stderr
11
macadamia% zsh -f
macadamia% precmd() { pwd>&11 }
/Users/schaefer
macadamia% 

Now here's 4.3.12-dev-1 started from 4.3.9:

macadamia% exec {stderr}>&2
macadamia% print $stderr
11
macadamia% Src/zsh -f
macadamia% precmd() { pwd>&11 }
/Users/schaefer/Documents/zsh                                                   
macadamia% 

So it's the calling shell rather than the called shell that is closing
a descriptor that it shouldn't.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Zsh 4.3.12: subshell in midnight commander:  precmd: 15: bad file descriptor
  2011-07-18 14:45     ` Bart Schaefer
@ 2011-07-18 15:24       ` Peter Stephenson
  2011-07-18 15:52         ` Bart Schaefer
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Stephenson @ 2011-07-18 15:24 UTC (permalink / raw)
  To: zsh-workers

On Mon, 18 Jul 2011 07:45:51 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> So it's the calling shell rather than the called shell that is closing
> a descriptor that it shouldn't.

I don't think so: both lsof and sh -c 'pwd >&11' agree that the FD is
still open in the lower shell.

See if this fixes it.

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.198
diff -p -u -r1.198 exec.c
--- Src/exec.c	23 Jun 2011 19:29:24 -0000	1.198
+++ Src/exec.c	18 Jul 2011 15:20:29 -0000
@@ -3008,11 +3008,17 @@ execcmd(Estate state, int input, int out
 		if (!checkclobberparam(fn))
 		    fil = -1;
 		else if (fn->fd2 > 9 &&
-			 (fn->fd2 > max_zsh_fd ||
-			  (fdtable[fn->fd2] != FDT_UNUSED &&
-			   fdtable[fn->fd2] != FDT_EXTERNAL) ||
-			  fn->fd2 == coprocin ||
-			  fn->fd2 == coprocout)) {
+			 /*
+			  * If the requested fd is > max_zsh_fd,
+			  * the shell doesn't know about it.
+			  * Just assume the user knows what they're
+			  * doing.
+			  */
+			 (fn->fd2 <= max_zsh_fd &&
+			  ((fdtable[fn->fd2] != FDT_UNUSED &&
+			    fdtable[fn->fd2] != FDT_EXTERNAL) ||
+			   fn->fd2 == coprocin ||
+			   fn->fd2 == coprocout))) {
 		    fil = -1;
 		    errno = EBADF;
 		} else {

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Zsh 4.3.12: subshell in midnight commander:  precmd: 15: bad file descriptor
  2011-07-18 15:24       ` Peter Stephenson
@ 2011-07-18 15:52         ` Bart Schaefer
  2011-07-18 16:01           ` Peter Stephenson
  2011-07-19  9:52           ` Peter Stephenson
  0 siblings, 2 replies; 14+ messages in thread
From: Bart Schaefer @ 2011-07-18 15:52 UTC (permalink / raw)
  To: zsh-workers

On Jul 18,  4:24pm, Peter Stephenson wrote:
} Subject: Re: Zsh 4.3.12: subshell in midnight commander:  precmd: 15: bad 
}
} On Mon, 18 Jul 2011 07:45:51 -0700
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} > So it's the calling shell rather than the called shell that is closing
} > a descriptor that it shouldn't.
} 
} I don't think so: both lsof and sh -c 'pwd >&11' agree that the FD is
} still open in the lower shell.

Hrm.  So why does it work when starting 4.3.12 from 4.3.9?  How is 11
getting to be <= max_zsh_fd inside 4.3.12 that case, whereas it is not
when the outer shell is 4.3.12?
 
} See if this fixes it.

It prevents the specific error in question, but there are also going to be
changes needed here:

   2975                         fn->fd1 = (int)getintvalue(v);
   2976                         if (errflag)
   2977                             bad = 1;
*  2978                         else if (fn->fd1 > max_zsh_fd)
   2979                             bad = 3;
   2980                         else if (fn->fd1 >= 10 &&
   2981                                  fdtable[fn->fd1] == FDT_INTERNAL)
   2982                             bad = 4;

And what's going to happen when max_zsh_fd does in fact become greater
than the "unknown" descriptor?  Any problems with fdtable[x] having
bogus data?  Isn't closem() going to attempt to close a descriptor
that it shouldn't?

I wonder if the underlying problem doesn't somehow stem from this:

      * 27721: Src/compat.c [with unnecessary test removed], Src/exec.c,
      Src/system.h, Src/utils.c:  update zopenmax() not to examine huge
      numbers of file descriptors; only call it at initialisation;
      rationalise use of fdtable_size and expansion of fdtable.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Zsh 4.3.12: subshell in midnight commander:  precmd: 15: bad file descriptor
  2011-07-18 15:52         ` Bart Schaefer
@ 2011-07-18 16:01           ` Peter Stephenson
  2011-07-18 16:26             ` Bart Schaefer
  2011-07-19  9:52           ` Peter Stephenson
  1 sibling, 1 reply; 14+ messages in thread
From: Peter Stephenson @ 2011-07-18 16:01 UTC (permalink / raw)
  To: zsh-workers

On Mon, 18 Jul 2011 08:52:34 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> Hrm.  So why does it work when starting 4.3.12 from 4.3.9?  How is 11
> getting to be <= max_zsh_fd inside 4.3.12 that case, whereas it is not
> when the outer shell is 4.3.12?

See what lsof says.  I suspect 4.3.9 is leaving open its SHTTY, since
setting FD_CLOEXEC on that is one of the changes I noted.  That would be
fd 12, so 11 is <max_zsh_fd.

> It prevents the specific error in question, but there are also going to be
> changes needed here:
> 
>    2975                         fn->fd1 = (int)getintvalue(v);
>    2976                         if (errflag)
>    2977                             bad = 1;
> *  2978                         else if (fn->fd1 > max_zsh_fd)
>    2979                             bad = 3;
>    2980                         else if (fn->fd1 >= 10 &&
>    2981                                  fdtable[fn->fd1] == FDT_INTERNAL)
>    2982                             bad = 4;

Hmm... that's saying we're closing an fd with {foo}>&- syntax that
we don't know about in the first place.  Do we just try to close it and
report an error closing an unknown fd if that fails?

> And what's going to happen when max_zsh_fd does in fact become greater
> than the "unknown" descriptor?  Any problems with fdtable[x] having
> bogus data?  Isn't closem() going to attempt to close a descriptor
> that it shouldn't?

I think that works, since we handle unknown fd's marked as unknown as
best we can.  In fact, I think you may just have tested it.

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Zsh 4.3.12: subshell in midnight commander:  precmd: 15: bad file descriptor
  2011-07-18 16:01           ` Peter Stephenson
@ 2011-07-18 16:26             ` Bart Schaefer
  2011-07-19  9:47               ` Peter Stephenson
  0 siblings, 1 reply; 14+ messages in thread
From: Bart Schaefer @ 2011-07-18 16:26 UTC (permalink / raw)
  To: zsh-workers

On Jul 18,  5:01pm, Peter Stephenson wrote:
} Subject: Re: Zsh 4.3.12: subshell in midnight commander:  precmd: 15: bad 
}
} On Mon, 18 Jul 2011 08:52:34 -0700
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} > Hrm.  So why does it work when starting 4.3.12 from 4.3.9?  How is 11
} > getting to be <= max_zsh_fd inside 4.3.12 that case, whereas it is not
} > when the outer shell is 4.3.12?
} 
} See what lsof says.  I suspect 4.3.9 is leaving open its SHTTY, since
} setting FD_CLOEXEC on that is one of the changes I noted.  That would be
} fd 12, so 11 is <max_zsh_fd.

According to lsof -p $$, the max FD is 10, not 12.

} > *  2978                         else if (fn->fd1 > max_zsh_fd)
} >    2979                             bad = 3;
} 
} Hmm... that's saying we're closing an fd with {foo}>&- syntax that
} we don't know about in the first place.  Do we just try to close it and
} report an error closing an unknown fd if that fails?

Hrm.  Right now it says

zsh: file descriptor 11 out of range, not closed

Strictly speaking there should be some way to close an arbitrary file
descriptor.  If the shell "knows about" all open descriptors that's
not an issue here ...

By the way, unrelated oddness:  Redirections are allowed to appear at
the start of a command like this:

    2>&- true

but

    {foo}>&- true

is a parse error.

-- 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Zsh 4.3.12: subshell in midnight commander:  precmd: 15: bad file descriptor
  2011-07-18 16:26             ` Bart Schaefer
@ 2011-07-19  9:47               ` Peter Stephenson
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Stephenson @ 2011-07-19  9:47 UTC (permalink / raw)
  To: zsh-workers

On Mon, 18 Jul 2011 09:26:18 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> } > *  2978                         else if (fn->fd1 > max_zsh_fd)
> } >    2979                             bad = 3;
> } 
> } Hmm... that's saying we're closing an fd with {foo}>&- syntax that
> } we don't know about in the first place.  Do we just try to close it and
> } report an error closing an unknown fd if that fails?
> 
> Hrm.  Right now it says
> 
> zsh: file descriptor 11 out of range, not closed
> 
> Strictly speaking there should be some way to close an arbitrary file
> descriptor.  If the shell "knows about" all open descriptors that's
> not an issue here ...

The following closes fd's it doesn't know about, but warns if the close
failed --- even if it does know about it, which is a change but seems
reasonable as the close was an explicit user request, particularly since
the failure to close doesn't cause the command to fail.  I haven't
turned this into a hard error, since that would be an incompatibility,
but arguably it should be.

The debug test in zclose() looked really screwy (it would lead to a
memory error as written), and doesn't make sense any more, so I've just
removed it rather than decoding it and fixing it.

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.199
diff -p -u -r1.199 exec.c
--- Src/exec.c	19 Jul 2011 09:26:57 -0000	1.199
+++ Src/exec.c	19 Jul 2011 09:45:19 -0000
@@ -2975,17 +2975,16 @@ execcmd(Estate state, int input, int out
 			fn->fd1 = (int)getintvalue(v);
 			if (errflag)
 			    bad = 1;
-			else if (fn->fd1 > max_zsh_fd)
-			    bad = 3;
-			else if (fn->fd1 >= 10 &&
+			else if (fn->fd1 <= max_zsh_fd) {
+			    if (fn->fd1 >= 10 &&
 				 fdtable[fn->fd1] == FDT_INTERNAL)
-			    bad = 4;
+				bad = 3;
+			}
 		    }
 		    if (bad) {
 			const char *bad_msg[] = {
 			    "parameter %s does not contain a file descriptor",
 			    "can't close file descriptor from readonly parameter %s",
-			    "file descriptor %d out of range, not closed",
 			    "file descriptor %d used by shell, not closed"
 			};
 			if (bad > 2)
@@ -2995,11 +2994,18 @@ execcmd(Estate state, int input, int out
 			execerr();
 		    }
 		}
+		/*
+		 * Note we may attempt to close an fd beyond max_zsh_fd:
+		 * OK as long as we never look in fdtable for it.
+ 		 */
 		if (!forked && fn->fd1 < 10 && save[fn->fd1] == -2)
 		    save[fn->fd1] = movefd(fn->fd1);
 		if (fn->fd1 < 10)
 		    closemn(mfds, fn->fd1);
-		zclose(fn->fd1);
+		if (zclose(fn->fd1) < 0) {
+		    zwarn("failed to close file descriptor %d: %e",
+			  fn->fd1, errno);
+		}
 		break;
 	    case REDIR_MERGEIN:
 	    case REDIR_MERGEOUT:
Index: Src/utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/utils.c,v
retrieving revision 1.259
diff -p -u -r1.259 utils.c
--- Src/utils.c	19 Jun 2011 16:26:11 -0000	1.259
+++ Src/utils.c	19 Jul 2011 09:45:19 -0000
@@ -1802,22 +1802,20 @@ zclose(int fd)
 {
     if (fd >= 0) {
 	/*
-	 * We sometimes zclose() an fd twice where the second
-	 * time is a catch-all in case there was a failure using
-	 * the fd.  This is harmless but we need to trap it
-	 * for the error check here.
+	 * Careful: we allow closing of arbitrary fd's, beyond
+	 * max_zsh_fd.  In that case we don't try anything clever.
 	 */
-	DPUTS2(fd > max_zsh_fd && fdtable[fd] != FDT_UNUSED,
-	       "BUG: fd is %d, max_zsh_fd is %d", fd, max_zsh_fd);
-	if (fdtable[fd] == FDT_FLOCK)
-	    fdtable_flocks--;
-	fdtable[fd] = FDT_UNUSED;
-	while (max_zsh_fd > 0 && fdtable[max_zsh_fd] == FDT_UNUSED)
-	    max_zsh_fd--;
-	if (fd == coprocin)
-	    coprocin = -1;
-	if (fd == coprocout)
-	    coprocout = -1;
+	if (fd <= max_zsh_fd) {
+	    if (fdtable[fd] == FDT_FLOCK)
+		fdtable_flocks--;
+	    fdtable[fd] = FDT_UNUSED;
+	    while (max_zsh_fd > 0 && fdtable[max_zsh_fd] == FDT_UNUSED)
+		max_zsh_fd--;
+	    if (fd == coprocin)
+		coprocin = -1;
+	    if (fd == coprocout)
+		coprocout = -1;
+	}
 	return close(fd);
     }
     return -1;
Index: Test/A04redirect.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/A04redirect.ztst,v
retrieving revision 1.18
diff -p -u -r1.18 A04redirect.ztst
--- Test/A04redirect.ztst	6 Mar 2011 21:37:38 -0000	1.18
+++ Test/A04redirect.ztst	19 Jul 2011 09:45:19 -0000
@@ -155,9 +155,12 @@
   (exec 3<&-
   read foo <&-)
 1:'<&-' redirection
+?(eval):1: failed to close file descriptor 3: bad file descriptor
+?(eval):2: failed to close file descriptor 0: bad file descriptor
 
   print foo >&-
 0:'>&-' redirection
+?(eval):1: failed to close file descriptor 1: bad file descriptor
 
   fn() { local foo; read foo; print $foo; }
   coproc fn

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Zsh 4.3.12: subshell in midnight commander:  precmd: 15: bad file descriptor
  2011-07-18 15:52         ` Bart Schaefer
  2011-07-18 16:01           ` Peter Stephenson
@ 2011-07-19  9:52           ` Peter Stephenson
  2011-07-19 14:28             ` Bart Schaefer
  1 sibling, 1 reply; 14+ messages in thread
From: Peter Stephenson @ 2011-07-19  9:52 UTC (permalink / raw)
  To: zsh-workers

On Mon, 18 Jul 2011 08:52:34 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> I wonder if the underlying problem doesn't somehow stem from this:
> 
>       * 27721: Src/compat.c [with unnecessary test removed], Src/exec.c,
>       Src/system.h, Src/utils.c:  update zopenmax() not to examine huge
>       numbers of file descriptors; only call it at initialisation;
>       rationalise use of fdtable_size and expansion of fdtable.

I skipped over this yesterday, but are you suggesting we call zopenmax()
with a larger limit when the user tries to manipulate an fd we don't
know about?  (Simply pushing the limit in zopenmax() up unconditionally
doesn't look like a robust fix.)

I'm not sure if we gain much with that:  we still have to trap the
points where we are passed a large enough fd, and until the shell wishes
to do something with the intervening values it has no particular need to
know about them.  But there could easily be a subtlety I've missed.

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Zsh 4.3.12: subshell in midnight commander:  precmd: 15: bad file descriptor
  2011-07-19  9:52           ` Peter Stephenson
@ 2011-07-19 14:28             ` Bart Schaefer
  0 siblings, 0 replies; 14+ messages in thread
From: Bart Schaefer @ 2011-07-19 14:28 UTC (permalink / raw)
  To: zsh-workers

On Jul 19, 10:52am, Peter Stephenson wrote:
} Subject: Re: Zsh 4.3.12: subshell in midnight commander:  precmd: 15: bad 
}
} On Mon, 18 Jul 2011 08:52:34 -0700
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} > I wonder if the underlying problem doesn't somehow stem from this:
} > 
} >       * 27721: Src/compat.c [with unnecessary test removed], Src/exec.c,
} >       Src/system.h, Src/utils.c:  update zopenmax() not to examine huge
} >       numbers of file descriptors; only call it at initialisation;
} >       rationalise use of fdtable_size and expansion of fdtable.
} 
} I skipped over this yesterday, but are you suggesting we call zopenmax()
} with a larger limit when the user tries to manipulate an fd we don't
} know about?

I was suggesting a cause rather than any specific solution; i.e., if the
old slow zopenmax() discovered descriptors which the new one does not,
then that might explain the change in behavior; and if so, then making
some sort of change to zopenmax() might head off other cases we haven't
found yet where the code assumes zopenmax() to be comprehensive.

If you think as of 29561 that you've caught them all, then that'll do.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Zsh 4.3.12: subshell in midnight commander:  precmd: 15: bad file descriptor
  2011-07-18 14:13 ` Peter Stephenson
@ 2011-10-02 19:00   ` Pavel Reznicek
  0 siblings, 0 replies; 14+ messages in thread
From: Pavel Reznicek @ 2011-10-02 19:00 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: zsh-workers


Hi,

   getting back to this issue, picking up the two patches mentioned in this 
thread:

http://www.zsh.org/mla/workers/2011/msg00988.html
http://www.zsh.org/mla/workers/2011/msg00994.html

and applying them on top of debian zsh 4.3.12 the problem really 
disappears.

Thanks a lot for the fix,
Pavel



On Mon, 18 Jul 2011, Peter Stephenson wrote:

> On Mon, 18 Jul 2011 10:26:27 +0200
> Pavel Reznicek <reznicek@ipnp.troja.mff.cuni.cz> wrote:
>>    after upgrading to zsh 4.3.12 (in debian), I am having problems running
>> midnight commander in zsh (going back to zsh version 4.3.10 it works
>> fine). The symptom is that when trying to use the subshell in mc, it
>> complains about:
>>
>> precmd: 15: bad file descriptor
>>
>> and the mc session get hanged. Of course it could be bug in mc and not
>> zsh, but since this behaviour is not seen with zsh 4.3.10, the zsh updates
>> since that version can tell what is going on. Browsing the mc source code,
>> the precmd command defined in there is:
>>
>> precmd(){ pwd>&%d;kill -STOP $$ }
>>
>> Would some have an idea why this precmd command would cause the error
>> message above in connection of the updateds between zsh 4.3.10 and 4.3.12
>> ?
>
> So if it's not (obviously) related to the "stop", we probably need to
> know what file descriptor is appearing there.  If it's 1 or 2 there's
> not obvious reason for parametrising it.  If it's not, it's not obvious
> what it would be --- something opened by mc, presumably, and possibly
> getting FD_CLOEXEC set, if the problem only occurs in subshells.
>
> Hmm... if it were the shell terminal output, this fix spotted by Bart
> might be relevant:
>
>    * 27134: Src/exec.c, Src/init.c: improve 27131 by setting
>      FD_CLOEXEC for SHTTY or closing it by hand when executing.
>
> but SHTTY isn't directly exposed to the shell user (which is why
> we're closing it).
>
> pws
>
>
> Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
> More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog
>


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2011-10-02 19:07 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-18  8:26 Zsh 4.3.12: subshell in midnight commander: precmd: 15: bad file descriptor Pavel Reznicek
2011-07-18  9:07 ` Peter Stephenson
2011-07-18 10:04   ` Pavel Reznicek
2011-07-18 14:07     ` Bart Schaefer
2011-07-18 14:45     ` Bart Schaefer
2011-07-18 15:24       ` Peter Stephenson
2011-07-18 15:52         ` Bart Schaefer
2011-07-18 16:01           ` Peter Stephenson
2011-07-18 16:26             ` Bart Schaefer
2011-07-19  9:47               ` Peter Stephenson
2011-07-19  9:52           ` Peter Stephenson
2011-07-19 14:28             ` Bart Schaefer
2011-07-18 14:13 ` Peter Stephenson
2011-10-02 19:00   ` Pavel Reznicek

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).