job table full.

job table full.

[Tanaka Akira]

I found a problem about job table with 3.1.5-pws-25.

Z(2):akr@is27e1u11% zsh -f
is27e1u11% while :
while> do
while> =echo $[i++]
while> done
0
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
zsh: job table full
is27e1u11% =echo $ZSH_VERSION
3.1.5-pws-25
is27e1u11% 

zsh should not overflow the table.
-- 
Tanaka Akira

Re: job table full.

[Peter Stephenson]

Tanaka Akira wrote:
> I found a problem about job table with 3.1.5-pws-25.
> zsh: job table full

Yes, this is what I found, but it's more serious than I realised.  Simply
using the shell for long enough will eventually hang it.  Looks like zsh is
the first shell with a job table leak.  Well, we've had everything else.

-- 
Peter Stephenson <pws@ibmth.df.unipi.it>       Tel: +39 050 844536
WWW:  http://www.ifh.de/~pws/
Dipartimento di Fisica, Via Buonarroti 2, 56127 Pisa, Italy

Re: Job table full

[Peter Stephenson]

Wayne Davison wrote:
> On Wed, May 12, 2004 at 02:40:29PM +0100, Peter Stephenson wrote:
> > A reliable way of reproducing it would help.
> 
> It's pretty easy:
> 
>     zsh -f
>     preexec() { x=`echo hi` }
>     sleep 1 &
>     :              [execute any 20 commands]
> 
> The problem appears to be that, after the job-control run, the value of
> "thisjob" in the preexec() function becomes -1.  This comparison then
> fails due to a signed/unsigned mismatch:
> 
>     if (thisjob >= jobtabsize - 1 && !expandjobtab()) {

Aha.  That's good.

thisjob is allowed to be -1, it simply means there's no valid
current job.  So the-test-that-no-one-understands should simply have
`thisjob != -1' in it, as in other places in the code.

Here is a fix with some other paranoid tests for thisjob when debugging
is turned on.

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.62
diff -u -r1.62 exec.c
--- Src/exec.c	21 May 2004 11:19:30 -0000	1.62
+++ Src/exec.c	25 May 2004 18:18:06 -0000
@@ -219,7 +219,7 @@
     /*
      * Is anybody willing to explain this test?
      */
-    if (thisjob >= jobtabsize - 1 && !expandjobtab()) {
+    if (thisjob != -1 && thisjob >= jobtabsize - 1 && !expandjobtab()) {
 	zerr("job table full", NULL, 0);
 	return -1;
     }
Index: Src/jobs.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/jobs.c,v
retrieving revision 1.28
diff -u -r1.28 jobs.c
--- Src/jobs.c	2 May 2004 19:55:58 -0000	1.28
+++ Src/jobs.c	25 May 2004 18:18:07 -0000
@@ -904,6 +904,7 @@
     Process pn, *pnlist;
     struct timezone dummy_tz;
 
+    DPUTS(thisjob == -1, "No valid job in addproc.");
     pn = (Process) zshcalloc(sizeof *pn);
     pn->pid = pid;
     if (text)
@@ -1035,6 +1036,7 @@
 waitjobs(void)
 {
     Job jn = jobtab + thisjob;
+    DPUTS(thisjob == -1, "No valid job in waitjobs.");
 
     if (jn->procs || jn->auxprocs)
 	zwaitjob(thisjob, 0);
@@ -1129,6 +1131,7 @@
 {
     Process pn;
 
+    DPUTS(thisjob == -1, "No valid job in spawnjob.");
     /* if we are not in a subshell */
     if (!subsh) {
 	if (curjob == -1 || !(jobtab[curjob].stat & STAT_STOPPED)) {

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************

Re: Job table full

[Wayne Davison]

On Tue, May 25, 2004 at 07:19:24PM +0100, Peter Stephenson wrote:
> So the-test-that-no-one-understands should simply have
> `thisjob != -1' in it, as in other places in the code.

I think it would also be good to change the type of jobtabsize and
maxjob from "size_t" to "int" so that we don't have a bunch of
signed/unsigned comparisons in the code.  Since other job-number
variables are ints, this should be fine.

..wayne..