Zsh Mailing List Archive
Messages sorted by:
Reverse Date,
Date,
Thread,
Author
Re: bug with eval, proc-subst and pipes
On Wed, 17 Jul 2013 00:00:27 -0700
Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> On Jul 16, 9:55pm, Peter Stephenson wrote:
> } It may therefore be a different problem from Stephane's.
>
> I don't think so, fundamentally. The problem in both cases is that the
> file descriptor for the process substitution is closed before the data
> is read. This has the unintended side-effect of removing the special
> device file. The reasons for the premature descriptor close may be
> different, but the end result is the same; I believe fixing either one
> would fix both.
We can see...
> } I suppose a smarter way of marking such file descriptors is needed,
> } perhaps one associated with the appropriate job in the same way as
> } auxiliary processes are remembered. Good luck avoiding leaks...
>
> Not in the way auxiliary processes are remembered; in the way temporary
> files are remembered. With #undef PATH_DEV_FD, zsh seems to do a very
> good job of cleaning up those FIFOs.
In that case, this patch does the exact equivalent. I've made the
entries in the temporary file list larger rather than adding a separate
entry to the job since it should use less memory in general.
It seems to fix the completely reproducible variant.
diff --git a/Src/exec.c b/Src/exec.c
index 75805d3..d462d97 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2860,9 +2860,6 @@ execcmd(Estate state, int input, int output, int how, int last1)
close(synch[1]);
read_loop(synch[0], &dummy, 1);
close(synch[0]);
-#ifdef PATH_DEV_FD
- closem(FDT_PROC_SUBST);
-#endif
if (how & Z_ASYNC) {
lastpid = (zlong) pid;
/* indicate it's possible to set status for lastpid */
@@ -3247,32 +3244,16 @@ execcmd(Estate state, int input, int output, int how, int last1)
if (is_shfunc) {
/* It's a shell function */
-#ifdef PATH_DEV_FD
- int i;
-
- for (i = 10; i <= max_zsh_fd; i++)
- if (fdtable[i] >= FDT_PROC_SUBST)
- fdtable[i]++;
-#endif
if (subsh_close >= 0)
zclose(subsh_close);
subsh_close = -1;
execshfunc((Shfunc) hn, args);
-#ifdef PATH_DEV_FD
- for (i = 10; i <= max_zsh_fd; i++)
- if (fdtable[i] >= FDT_PROC_SUBST)
- if (--(fdtable[i]) <= FDT_PROC_SUBST)
- zclose(i);
-#endif
} else {
/* It's a builtin */
if (forked)
closem(FDT_INTERNAL);
lastval = execbuiltin(args, (Builtin) hn);
-#ifdef PATH_DEV_FD
- closem(FDT_PROC_SUBST);
-#endif
fflush(stdout);
if (save[1] == -2) {
if (ferror(stdout)) {
@@ -3887,9 +3868,7 @@ getoutputfile(char *cmd, char **eptr)
untokenize(s);
}
- if (!jobtab[thisjob].filelist)
- jobtab[thisjob].filelist = znewlinklist();
- zaddlinknode(jobtab[thisjob].filelist, nam);
+ addfilelist(nam, 0);
if (!s)
child_block();
@@ -3975,9 +3954,7 @@ getproc(char *cmd, char **eptr)
return NULL;
if (!(prog = parsecmd(cmd, eptr)))
return NULL;
- if (!jobtab[thisjob].filelist)
- jobtab[thisjob].filelist = znewlinklist();
- zaddlinknode(jobtab[thisjob].filelist, ztrdup(pnam));
+ addfilelist(pnam, 0);
if ((pid = zfork(&bgtime))) {
if (pid == -1)
@@ -3995,7 +3972,7 @@ getproc(char *cmd, char **eptr)
entersubsh(ESUB_ASYNC|ESUB_PGRP);
redup(fd, out);
#else /* PATH_DEV_FD */
- int pipes[2];
+ int pipes[2], fd;
if (thisjob == -1)
return NULL;
@@ -4012,7 +3989,9 @@ getproc(char *cmd, char **eptr)
zclose(pipes[!out]);
return NULL;
}
- fdtable[pipes[!out]] = FDT_PROC_SUBST;
+ fd = pipes[!out];
+ fdtable[fd] = FDT_PROC_SUBST;
+ addfilelist(NULL, fd);
if (!out)
{
addproc(pid, NULL, 1, &bgtime);
diff --git a/Src/jobs.c b/Src/jobs.c
index 0dbb10b..a1955bb 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -1113,16 +1113,48 @@ printjob(Job jn, int lng, int synch)
return doneprint;
}
+/* Add a file to be deleted or fd to be closed to the current job */
+
+/**/
+void
+addfilelist(const char *name, int fd)
+{
+ Jobfile jf = (Jobfile)zalloc(sizeof(struct jobfile));
+ LinkList ll = jobtab[thisjob].filelist;
+
+ if (!ll)
+ ll = jobtab[thisjob].filelist = znewlinklist();
+ if (name)
+ {
+ jf->u.name = ztrdup(name);
+ jf->is_fd = 0;
+ }
+ else
+ {
+ jf->u.fd = fd;
+ jf->is_fd = 1;
+ }
+ zaddlinknode(ll, jf);
+}
+
+/* Finished with list of files for a job */
+
/**/
void
deletefilelist(LinkList file_list, int disowning)
{
- char *s;
+ Jobfile jf;
if (file_list) {
- while ((s = (char *)getlinknode(file_list))) {
- if (!disowning)
- unlink(s);
- zsfree(s);
+ while ((jf = (Jobfile)getlinknode(file_list))) {
+ if (jf->is_fd) {
+ if (!disowning)
+ zclose(jf->u.fd);
+ } else {
+ if (!disowning)
+ unlink(jf->u.name);
+ zsfree(jf->u.name);
+ }
+ zfree(jf, sizeof(*jf));
}
zfree(file_list, sizeof(struct linklist));
}
diff --git a/Src/zsh.h b/Src/zsh.h
index 299357d..ebd3cb7 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -374,9 +374,8 @@ enum {
#ifdef PATH_DEV_FD
/*
* Entry used by a process substition.
- * The value will be incremented on entering a function and
- * decremented on exit; we don't close entries greater than
- * FDT_PROC_SUBST except when closing everything.
+ * This marker is not tested internally as we associated the file
+ * descriptor with a job for closing.
*/
#define FDT_PROC_SUBST 6
#endif
@@ -422,6 +421,7 @@ typedef struct heap *Heap;
typedef struct heapstack *Heapstack;
typedef struct histent *Histent;
typedef struct hookdef *Hookdef;
+typedef struct jobfile *Jobfile;
typedef struct job *Job;
typedef struct linkedmod *Linkedmod;
typedef struct linknode *LinkNode;
@@ -878,6 +878,18 @@ struct eccstr {
/* Definitions for job table and job control */
/********************************************/
+/* Entry in filelist linked list in job table */
+
+struct jobfile {
+ /* Record to be deleted or closed */
+ union {
+ char *name; /* Name of file to delete */
+ int fd; /* File descriptor to close */
+ } u;
+ /* Discriminant */
+ int is_fd;
+};
+
/* entry in the job table */
struct job {
@@ -889,6 +901,7 @@ struct job {
struct process *procs; /* list of processes */
struct process *auxprocs; /* auxiliary processes e.g multios */
LinkList filelist; /* list of files to delete when done */
+ /* elements are struct jobfile */
int stty_in_env; /* if STTY=... is present */
struct ttyinfo *ty; /* the modes specified by STTY */
};
--
Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/
Messages sorted by:
Reverse Date,
Date,
Thread,
Author