From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.bugs Subject: bug#23808: Emacs 25 open-network-stream, make-network-process Date: Thu, 11 Aug 2016 02:14:18 -0700 Organization: UCLA Computer Science Department Message-ID: <4dc79eb4-e2b6-9f33-cf61-af1971e10275@cs.ucla.edu> References: <5767C0A3.9010902@cs.ucla.edu> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------4868CA9D6499EF0D7516D1E0" X-Trace: blaine.gmane.org 1470906923 7097 195.159.176.226 (11 Aug 2016 09:15:23 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Thu, 11 Aug 2016 09:15:23 +0000 (UTC) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 Cc: Lars Ingebrigtsen To: 23808@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Thu Aug 11 11:15:19 2016 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bXm59-0001hb-8w for geb-bug-gnu-emacs@m.gmane.org; Thu, 11 Aug 2016 11:15:19 +0200 Original-Received: from localhost ([::1]:47040 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bXm56-00072h-80 for geb-bug-gnu-emacs@m.gmane.org; Thu, 11 Aug 2016 05:15:16 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:51353) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bXm4x-0006xx-8L for bug-gnu-emacs@gnu.org; Thu, 11 Aug 2016 05:15:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bXm4s-0006yM-UJ for bug-gnu-emacs@gnu.org; Thu, 11 Aug 2016 05:15:06 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:55258) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bXm4s-0006yI-Q9 for bug-gnu-emacs@gnu.org; Thu, 11 Aug 2016 05:15:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1bXm4s-0004Di-Ds for bug-gnu-emacs@gnu.org; Thu, 11 Aug 2016 05:15:02 -0400 X-Loop: help-debbugs@gnu.org In-Reply-To: <5767C0A3.9010902@cs.ucla.edu> Resent-From: Paul Eggert Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 11 Aug 2016 09:15:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 23808 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 23808-submit@debbugs.gnu.org id=B23808.147090686816164 (code B ref 23808); Thu, 11 Aug 2016 09:15:02 +0000 Original-Received: (at 23808) by debbugs.gnu.org; 11 Aug 2016 09:14:28 +0000 Original-Received: from localhost ([127.0.0.1]:52970 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bXm4K-0004Ce-1v for submit@debbugs.gnu.org; Thu, 11 Aug 2016 05:14:28 -0400 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:44187) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bXm4I-0004CQ-0S for 23808@debbugs.gnu.org; Thu, 11 Aug 2016 05:14:26 -0400 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 0BD3516112C; Thu, 11 Aug 2016 02:14:20 -0700 (PDT) Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id pZlqeeJi6YzU; Thu, 11 Aug 2016 02:14:18 -0700 (PDT) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id C5C6B161135; Thu, 11 Aug 2016 02:14:18 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id Ic6N_xeciyRI; Thu, 11 Aug 2016 02:14:18 -0700 (PDT) Original-Received: from [192.168.1.9] (unknown [100.32.155.148]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id A684E161117; Thu, 11 Aug 2016 02:14:18 -0700 (PDT) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:122061 Archived-At: This is a multi-part message in MIME format. --------------4868CA9D6499EF0D7516D1E0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable [CC'ing this to Lars, since this bug seems to have been introduced by the= async=20 DNS changes in February.] Attached is a proposed patch for Bug#23808. The patch attempts to fix the= bug=20 reported by my anonymous correspondent, who writes "It appears that befor= e the=20 refactoring, make-network-process would not create a process object if th= e=20 socket connection failed, or if it had been interrupted by ^G. However af= ter the=20 new refactoring, the process object is created upfront with status 'run',= before=20 the connection is attempted. The refactoring failed to preserve the seman= tics." --------------4868CA9D6499EF0D7516D1E0 Content-Type: text/x-diff; name="0001-Fix-process-leak-with-make-network-process.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="0001-Fix-process-leak-with-make-network-process.patch" =46rom a21dfe515552c7d7f486950e9231b8ca2668f6cc Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Thu, 11 Aug 2016 01:56:16 -0700 Subject: [PATCH] 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. --- src/process.c | 49 ++++++++++++++++++------------------------------- src/process.h | 9 +++++---- 2 files changed, 23 insertions(+), 35 deletions(-) diff --git a/src/process.c b/src/process.c index fb32698..69d1b2a 100644 --- a/src/process.c +++ b/src/process.c @@ -238,6 +238,7 @@ static int process_output_delay_count; =20 static bool process_output_skip; =20 +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); =20 /* Mask of bits indicating the descriptors that we wait for input on. *= / =20 @@ -1407,8 +1405,6 @@ DEFUN ("process-list", Fprocess_list, Sprocess_list= , 0, 0, 0, =0C /* Starting asynchronous inferior processes. */ =20 -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 obje= ct for it. =20 @@ -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 =3D SPECPDL_INDEX (); - USE_SAFE_ALLOCA; =20 if (nargs =3D=3D 0) return Qnil; @@ -1508,10 +1503,6 @@ usage: (make-process &rest ARGS) */) } =20 proc =3D 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); =20 pset_childp (XPROCESS (proc), Qt); @@ -1561,6 +1552,8 @@ usage: (make-process &rest ARGS) */) BUF_ZV (XBUFFER (buffer)), BUF_ZV_BYTE (XBUFFER (buffer))); =20 + 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); } =20 -/* 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 <=3D 0 && XPROCESS (proc)->pid !=3D -2) remove_process (proc); } @@ -3124,7 +3110,6 @@ connect_network_socket (Lisp_Object proc, Lisp_Obje= ct addrinfos, Lisp_Object use_external_socket_p) { ptrdiff_t count =3D SPECPDL_INDEX (); - ptrdiff_t count1; int s =3D -1, outch, inch; int xerrno =3D 0; int family; @@ -3145,7 +3130,6 @@ connect_network_socket (Lisp_Object proc, Lisp_Obje= ct addrinfos, } =20 /* Do this in case we never enter the while-loop below. */ - count1 =3D SPECPDL_INDEX (); s =3D -1; =20 while (!NILP (addrinfos)) @@ -3313,7 +3297,7 @@ connect_network_socket (Lisp_Object proc, Lisp_Obje= ct addrinfos, immediate_quit =3D 0; =20 /* Discard the unwind protect closing S. */ - specpdl_ptr =3D specpdl + count1; + specpdl_ptr =3D specpdl + count; emacs_close (s); s =3D -1; if (0 <=3D socket_to_use) @@ -3398,10 +3382,7 @@ connect_network_socket (Lisp_Object proc, Lisp_Obj= ect addrinfos, p->outfd =3D outch; =20 /* Discard the unwind protect for closing S, if any. */ - specpdl_ptr =3D specpdl + count1; - - /* Unwind bind_polling_period and request_sigio. */ - unbind_to (count, Qnil); + specpdl_ptr =3D specpdl + count; =20 if (p->is_server && p->socktype !=3D SOCK_DGRAM) pset_status (p, Qlisten); @@ -3925,7 +3906,12 @@ usage: (make-network-process &rest ARGS) */) =20 if (!NILP (buffer)) buffer =3D Fget_buffer_create (buffer); + + /* Unwind bind_polling_period. */ + unbind_to (count, Qnil); + proc =3D make_process (name); + record_unwind_protect (remove_process, proc); p =3D 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) */) =20 set_network_socket_coding_system (proc, host, service, name); =20 - unbind_to (count, Qnil); - /* :server BOOL */ tem =3D 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 =3D true; =20 + bool postpone_connection =3D 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 =3D dns_request; p->status =3D list1 (Qconnect); - return proc; + postpone_connection =3D true; } #endif + if (! postpone_connection) + connect_network_socket (proc, addrinfos, use_external_socket_p); =20 - connect_network_socket (proc, addrinfos, use_external_socket_p); + specpdl_ptr =3D specpdl + count; return proc; } =20 diff --git a/src/process.h b/src/process.h index 6c227bc..9926050 100644 --- a/src/process.h +++ b/src/process.h @@ -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. */ =20 - /* 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 connection= s, + 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; --=20 2.5.5 --------------4868CA9D6499EF0D7516D1E0--