From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Robert Pluim Newsgroups: gmane.emacs.devel Subject: Re: [RFC] automatically retrying network connections Date: Sun, 22 Jul 2018 12:41:53 +0200 Message-ID: <87fu0bft72.fsf@gmail.com> References: <87sh4dfz8r.fsf@gmail.com> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: blaine.gmane.org 1532256014 10901 195.159.176.226 (22 Jul 2018 10:40:14 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sun, 22 Jul 2018 10:40:14 +0000 (UTC) Cc: emacs-devel@gnu.org To: Lars Ingebrigtsen Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Jul 22 12:40:09 2018 Return-path: Envelope-to: ged-emacs-devel@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 1fhBmb-0002jK-Lq for ged-emacs-devel@m.gmane.org; Sun, 22 Jul 2018 12:40:09 +0200 Original-Received: from localhost ([::1]:55500 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fhBog-0003yE-Mf for ged-emacs-devel@m.gmane.org; Sun, 22 Jul 2018 06:42:18 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:55242) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fhBoW-0003xw-Dn for emacs-devel@gnu.org; Sun, 22 Jul 2018 06:42:12 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fhBoR-0001SF-FV for emacs-devel@gnu.org; Sun, 22 Jul 2018 06:42:08 -0400 Original-Received: from mail-wr1-x436.google.com ([2a00:1450:4864:20::436]:36852) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fhBoR-0001Rz-2g for emacs-devel@gnu.org; Sun, 22 Jul 2018 06:42:03 -0400 Original-Received: by mail-wr1-x436.google.com with SMTP id h9-v6so15137075wro.3 for ; Sun, 22 Jul 2018 03:42:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:mail-followup-to:mail-copies-to :gmane-reply-to-list:date:in-reply-to:message-id:mime-version; bh=wibq+lvgpB1pFRfc0WGQ1ulbD5j+f4oI5tLv3cYxxaw=; b=m5nl1BXyFyRVTW/idkEZhOsuavgOWyMZJeFNssOjdlBY9nfA0GVlQXF9TjIPvSGxQF HwSv3WECbCa+3vtOq6/K3h94DmF9lvl2yQVdh/HkUxDj97gOsgSFcoC3j5hyquW54635 7Xn/Agd+23wAXD55Yrd3AOBdxbDhXSJ1PCq2TNenrL578Z3WW3jToVerHW8AgOISd6qx ABxlEC6v28JHdrsQyP77pm4kyMM0P7OyF6xxDId7ypmN+3yK2zubDPXSgO2Q2hHJCCQs n+5bD5o6h7cg2cZQNKHBMg775Hyv+xDN8HNZ9rdLHbfEA2JWOh9cQa6d0u1qG989bG0E 2D8w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:mail-followup-to :mail-copies-to:gmane-reply-to-list:date:in-reply-to:message-id :mime-version; bh=wibq+lvgpB1pFRfc0WGQ1ulbD5j+f4oI5tLv3cYxxaw=; b=Wb/KtMC45zkkWbCIAyPQ7SOxtSo9Z2EGwbs7iDpfqL87nVWKuuAnx30BQRZ+yDV0E7 LrILVy4kh5EbVXQ+oOwzfcfGJgjgxytBn3MZsuAlYFW/4oPMWcfqWpbEqynYDqtdcB/E i44DS1W86XXQHPwd4BrbzA3I/umZLowEo6QnNxzYqatT4RnN9AYyfAvyy0tXsZh10J9r mucVwa7+DPPhAYa8AyO7QJactY0/x4nkwyRLX3qKZNRVV20dTdE+Q2NQAjM+rA/CDllw 5HlxgjrzANJer73RkBtKeq491oIosJKAffQY2Nvf1CEDrMDeCLSvly2lVZUaiGdXnnu1 lQEA== X-Gm-Message-State: AOUpUlHt+HDeThaEIUqY3nlwKi4YRuqptlnQPsQ4beBTvK4TolffXvJ6 hvuSqr8/uSBih9FoN0pzSmHdCRQ+ X-Google-Smtp-Source: AAOMgpcG7ASL5L0yh9OkJ3nYP2ymErWxzkesoNObS7Z8HAdULZNI0ZBnftFkGO7N6tWBRHqn+uKCdA== X-Received: by 2002:adf:9546:: with SMTP id 64-v6mr5524810wrs.257.1532256121539; Sun, 22 Jul 2018 03:42:01 -0700 (PDT) Original-Received: from rpluim-ubuntu (vav06-1-78-207-202-134.fbx.proxad.net. [78.207.202.134]) by smtp.gmail.com with ESMTPSA id h5-v6sm4986022wre.46.2018.07.22.03.41.54 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 22 Jul 2018 03:41:55 -0700 (PDT) Mail-Followup-To: emacs-devel@gnu.org Mail-Copies-To: never Gmane-Reply-To-List: yes In-Reply-To: (Lars Ingebrigtsen's message of "Sun, 22 Jul 2018 12:24:31 +0200") X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:4864:20::436 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:227656 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Lars Ingebrigtsen writes: > Robert Pluim writes: > >> one of the consequences of asking the user questions during network >> connection setup is that the server they're trying to connect to might >> have decided to close the connection by the time they've finished >> answering. > > Yes, the NSM should reconnect if the user says "go ahead" to the NSM > warning and the server has closed the connection. The reason that > landed on the back burner is that servers seem to mostly have long > timeouts, so this turned out to be less of a problem in practice than I > expected. (I.e., I don't recall seeing a bug report about this, which > is just plain weird.) There=CA=BCs this little-known news server called news.gmane.org that has a 10 second timeout :-) Reconnecting immediately works, so it=CA=BCs not a big deal. So you'd put this in nsm-verify-connection, without a user option? I don=CA=BCt think nsm currently has access to all the connection setup parameters, which is why I put the logic in open-network-stream. Proof-of-concept patch attached (it should really do more checking of what nsm returned). > We could also reverse the logic a bit and have the NSM always shut down > the connection before prompting. This will make network connections > (that prompt an NSM warning) be somewhat slower, but it shouldn't be a > big deal. That seems wasteful. In most cases the connection will complete OK, so why add an extra connection setup for all cases? > In any case, when reconnecting we have to consider whether this would > trigger extra auth-source prompts, which would be annoying, but I have a > feeling like these are mostly done later in the connection process > usually, so it shouldn't be an issue. If you=CA=BCre thinking of authentication for SMTP sessions and the like, they'd all arrive after completion of the TLS setup. Robert --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-Allow-retrying-of-network-connections-on-failure.patch >From 99ba6345bf38305b5480d07851bc5d64fd3e9dcd Mon Sep 17 00:00:00 2001 From: Robert Pluim Date: Fri, 20 Jul 2018 18:35:22 +0200 Subject: [PATCH] Allow retrying of network connections on failure To: emacs-devel@gnu.org With network-security-manager, there are cases where the user is expected to answer one or more questions before a network connection is finally established, which can result in the other end having timed out. Allow specifying :retry-on-fail t to open-network-stream to retry the connection, which should now succeed. * src/process.c (syms_of_process): Define Qprocess_not_running_error symbol. (send_process): Signal Qprocess_not_running_error for the specific case of the process having died only. * lisp/net/network-stream.el (open-network-stream): Add :retry-on-fail parameter, and retry network connection if it is set and we receive a Qprocess_not_running_error. --- lisp/net/network-stream.el | 42 +++++++++++++++++++++++++++----------- src/process.c | 15 +++++++++++++- 2 files changed, 44 insertions(+), 13 deletions(-) diff --git a/lisp/net/network-stream.el b/lisp/net/network-stream.el index a0589e25a4..3ecd8c2c64 100644 --- a/lisp/net/network-stream.el +++ b/lisp/net/network-stream.el @@ -153,22 +153,34 @@ open-network-stream opening a TLS connection. The first element is the TLS type (either `gnutls-x509pki' or `gnutls-anon'), and the remaining elements should be a keyword list accepted by -gnutls-boot (as returned by `gnutls-boot-parameters')." +gnutls-boot (as returned by `gnutls-boot-parameters'). + +:retry-on-fail, if non-nil, means attempt the connection again, +once, if it initially fails. This is to cover the case where +answering network-security-manager questions causes the remote +server to timeout the connection." (unless (featurep 'make-network-process) (error "Emacs was compiled without networking support")) (let ((type (plist-get parameters :type)) - (return-list (plist-get parameters :return-list))) + (return-list (plist-get parameters :return-list)) + (fun 'make-network-process) + (args (list :name name :buffer buffer + :host (puny-encode-domain host) :service service + :nowait (plist-get parameters :nowait) + :tls-parameters + (plist-get parameters :tls-parameters)))) (if (and (not return-list) (or (eq type 'plain) (and (memq type '(nil network)) (not (and (plist-get parameters :success) (plist-get parameters :capability-command)))))) ;; The simplest case: wrapper around `make-network-process'. - (make-network-process :name name :buffer buffer - :host (puny-encode-domain host) :service service - :nowait (plist-get parameters :nowait) - :tls-parameters - (plist-get parameters :tls-parameters)) + (condition-case err + (apply fun args) + (process-not-running-error + (when (plist-get parameters :retry-on-fail) + (delete-process (cdr err)) + (apply fun args)))) (let ((work-buffer (or buffer (generate-new-buffer " *stream buffer*"))) (fun (cond ((and (eq type 'plain) @@ -181,12 +193,18 @@ open-network-stream ((eq type 'shell) 'network-stream-open-shell) (t (error "Invalid connection type %s" type)))) result) - (unwind-protect + (condition-case err (setq result (funcall fun name work-buffer host service parameters)) - (unless buffer - (and (processp (car result)) - (set-process-buffer (car result) nil)) - (kill-buffer work-buffer))) + (process-not-running-error + (when (plist-get parameters :retry-on-fail) + (delete-process (cdr err)) + (setq result (funcall fun name work-buffer host service parameters)))) + (error + (unless buffer + (and (processp (car result)) + (set-process-buffer (car result) nil)) + (kill-buffer work-buffer)) + (signal (car err) (cdr err)))) (if return-list (list (car result) :greeting (nth 1 result) diff --git a/src/process.c b/src/process.c index 279b74bc66..222bd9fcff 100644 --- a/src/process.c +++ b/src/process.c @@ -6233,8 +6233,11 @@ send_process (Lisp_Object proc, const char *buf, ptrdiff_t len, if (p->raw_status_new) update_status (p); + /* Process might have died whilst we were waiting for the user to + answer nsm questions. Signal a specific error in that case so + higher levels can retry if they want. */ if (! EQ (p->status, Qrun)) - error ("Process %s not running", SDATA (p->name)); + xsignal (Qprocess_not_running_error, proc); if (p->outfd < 0) error ("Output file descriptor of %s is closed", SDATA (p->name)); @@ -8091,6 +8094,8 @@ syms_of_process (void) { #ifdef subprocesses + Lisp_Object error_tail; + DEFSYM (Qprocessp, "processp"); DEFSYM (Qrun, "run"); DEFSYM (Qstop, "stop"); @@ -8241,6 +8246,14 @@ returns non-`nil'. */); "internal-default-interrupt-process"); DEFSYM (Qinterrupt_process_functions, "interrupt-process-functions"); + DEFSYM (Qprocess_not_running_error, "process-not-running-error"); + error_tail = pure_cons (Qprocess_not_running_error, Qnil); + + Fput (Qprocess_not_running_error, Qerror_conditions, + error_tail); + Fput (Qprocess_not_running_error, Qerror_message, + build_pure_c_string ("process not running error")); + defsubr (&Sprocessp); defsubr (&Sget_process); defsubr (&Sdelete_process); -- 2.18.0.129.ge3331758f1 --=-=-=--