Commit 2dd8044b authored by Paul Eggert's avatar Paul Eggert

Fix process leak with make-network-process

This problem was introduced by the recent async changes (Bug#23808).
* src/process.c (Fmake_process): Move USE_SAFE_ALLOCA later,
so that it follows the start_process_unwind unwind-protect.
Set pid to -1 while the process is being created.
(start_process_unwind): Omit unnecessary emacs_abort test.
(connect_network_socket): Simplify use of counts.  Unwind
bind_polling_period a bit earlier, so that a remove_process
unwind-protect can be added when needed; this is the heart of
the fix.  Undo the unwind-protect just before returning.
parent 2e0a2b37
......@@ -238,6 +238,7 @@ static int process_output_delay_count;
static bool process_output_skip;
static void start_process_unwind (Lisp_Object);
static void create_process (Lisp_Object, char **, Lisp_Object);
#ifdef USABLE_SIGIO
static bool keyboard_bit_set (fd_set *);
......@@ -245,11 +246,8 @@ static bool keyboard_bit_set (fd_set *);
static void deactivate_process (Lisp_Object);
static int status_notify (struct Lisp_Process *, struct Lisp_Process *);
static int read_process_output (Lisp_Object, int);
static void handle_child_signal (int);
static void create_pty (Lisp_Object);
static Lisp_Object get_process (register Lisp_Object name);
static void exec_sentinel (Lisp_Object proc, Lisp_Object reason);
static void exec_sentinel (Lisp_Object, Lisp_Object);
/* Mask of bits indicating the descriptors that we wait for input on. */
......@@ -1407,8 +1405,6 @@ DEFUN ("process-list", Fprocess_list, Sprocess_list, 0, 0, 0,
/* Starting asynchronous inferior processes. */
static void start_process_unwind (Lisp_Object proc);
DEFUN ("make-process", Fmake_process, Smake_process, 0, MANY, 0,
doc: /* Start a program in a subprocess. Return the process object for it.
......@@ -1459,7 +1455,6 @@ usage: (make-process &rest ARGS) */)
Lisp_Object buffer, name, command, program, proc, contact, current_dir, tem;
Lisp_Object xstderr, stderrproc;
ptrdiff_t count = SPECPDL_INDEX ();
USE_SAFE_ALLOCA;
if (nargs == 0)
return Qnil;
......@@ -1508,10 +1503,6 @@ usage: (make-process &rest ARGS) */)
}
proc = make_process (name);
/* If an error occurs and we can't start the process, we want to
remove it from the process list. This means that each error
check in create_process doesn't need to call remove_process
itself; it's all taken care of here. */
record_unwind_protect (start_process_unwind, proc);
pset_childp (XPROCESS (proc), Qt);
......@@ -1561,6 +1552,8 @@ usage: (make-process &rest ARGS) */)
BUF_ZV (XBUFFER (buffer)),
BUF_ZV_BYTE (XBUFFER (buffer)));
USE_SAFE_ALLOCA;
{
/* Decide coding systems for communicating with the process. Here
we don't setup the structure coding_system nor pay attention to
......@@ -1719,18 +1712,11 @@ usage: (make-process &rest ARGS) */)
return unbind_to (count, proc);
}
/* This function is the unwind_protect form for Fstart_process. If
PROC doesn't have its pid set, then we know someone has signaled
an error and the process wasn't started successfully, so we should
remove it from the process list. */
/* If PROC doesn't have its pid set, then an error was signaled and
the process wasn't started successfully, so remove it. */
static void
start_process_unwind (Lisp_Object proc)
{
if (!PROCESSP (proc))
emacs_abort ();
/* Was PROC started successfully?
-2 is used for a pty with no process, eg for gdb. */
if (XPROCESS (proc)->pid <= 0 && XPROCESS (proc)->pid != -2)
remove_process (proc);
}
......@@ -3124,7 +3110,6 @@ connect_network_socket (Lisp_Object proc, Lisp_Object addrinfos,
Lisp_Object use_external_socket_p)
{
ptrdiff_t count = SPECPDL_INDEX ();
ptrdiff_t count1;
int s = -1, outch, inch;
int xerrno = 0;
int family;
......@@ -3145,7 +3130,6 @@ connect_network_socket (Lisp_Object proc, Lisp_Object addrinfos,
}
/* Do this in case we never enter the while-loop below. */
count1 = SPECPDL_INDEX ();
s = -1;
while (!NILP (addrinfos))
......@@ -3313,7 +3297,7 @@ connect_network_socket (Lisp_Object proc, Lisp_Object addrinfos,
immediate_quit = 0;
/* Discard the unwind protect closing S. */
specpdl_ptr = specpdl + count1;
specpdl_ptr = specpdl + count;
emacs_close (s);
s = -1;
if (0 <= socket_to_use)
......@@ -3398,10 +3382,7 @@ connect_network_socket (Lisp_Object proc, Lisp_Object addrinfos,
p->outfd = outch;
/* Discard the unwind protect for closing S, if any. */
specpdl_ptr = specpdl + count1;
/* Unwind bind_polling_period and request_sigio. */
unbind_to (count, Qnil);
specpdl_ptr = specpdl + count;
if (p->is_server && p->socktype != SOCK_DGRAM)
pset_status (p, Qlisten);
......@@ -3925,7 +3906,12 @@ usage: (make-network-process &rest ARGS) */)
if (!NILP (buffer))
buffer = Fget_buffer_create (buffer);
/* Unwind bind_polling_period. */
unbind_to (count, Qnil);
proc = make_process (name);
record_unwind_protect (remove_process, proc);
p = XPROCESS (proc);
pset_childp (p, contact);
pset_plist (p, Fcopy_sequence (Fplist_get (contact, QCplist)));
......@@ -3956,8 +3942,6 @@ usage: (make-network-process &rest ARGS) */)
set_network_socket_coding_system (proc, host, service, name);
unbind_to (count, Qnil);
/* :server BOOL */
tem = Fplist_get (contact, QCserver);
if (!NILP (tem))
......@@ -3974,6 +3958,7 @@ usage: (make-network-process &rest ARGS) */)
&& !NILP (Fplist_get (contact, QCnowait)))
p->is_non_blocking_client = true;
bool postpone_connection = false;
#ifdef HAVE_GETADDRINFO_A
/* With async address resolution, the list of addresses is empty, so
postpone connecting to the server. */
......@@ -3981,11 +3966,13 @@ usage: (make-network-process &rest ARGS) */)
{
p->dns_request = dns_request;
p->status = list1 (Qconnect);
return proc;
postpone_connection = true;
}
#endif
if (! postpone_connection)
connect_network_socket (proc, addrinfos, use_external_socket_p);
connect_network_socket (proc, addrinfos, use_external_socket_p);
specpdl_ptr = specpdl + count;
return proc;
}
......
......@@ -118,10 +118,11 @@ struct Lisp_Process
/* After this point, there are no Lisp_Objects any more. */
/* alloc.c assumes that `pid' is the first such non-Lisp slot. */
/* Number of this process.
allocate_process assumes this is the first non-Lisp_Object field.
A value 0 is used for pseudo-processes such as network or serial
connections. */
/* Process ID. A positive value is a child process ID.
Zero is for pseudo-processes such as network or serial connections,
or for processes that have not been fully created yet.
-1 is for a process that was not created successfully.
-2 is for a pty with no process, e.g., for GDB. */
pid_t pid;
/* Descriptor by which we read from this process. */
int infd;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment