bug#5650: 23.1; ERC causes Emacs to hang on network dropouts

bug#5650: 23.1; ERC causes Emacs to hang on network dropouts

[Richard Lewis]

When my network connection drops out, ERC loses its connection and
attempts to reconnect. I see the message "connecting to server
irc.freenode.net 6777..." in the minibuffer and Emacs becomes
unresponsive.

netstat tells me that the Emacs process has a TCP socket in the
SYN_SENT state to freenode.net.

I found today that, if left for a long time (not sure how long, but <
2 hours [which was the amount of time I left my computer unattended]
and > ~10 minutes [which was the amount of time I fiddled with it last
time it happened]), it evetually gives up and Emacs becomes responsive
again.

In GNU Emacs 23.1.1 (i486-pc-linux-gnu, GTK+ Version 2.18.2)
 of 2010-01-27 on raven, modified by Debian
Windowing system distributor `The X.Org Foundation', version 11.0.10705000
configured using `configure  '--build=i486-linux-gnu' '--host=i486-linux-gnu' '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--localstatedir=/var/lib' '--infodir=/usr/share/info' '--mandir=/usr/share/man' '--with-pop=yes' '--enable-locallisppath=/etc/emacs23:/etc/emacs:/usr/local/share/emacs/23.1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/23.1/site-lisp:/usr/share/emacs/site-lisp:/usr/share/emacs/23.1/leim' '--with-x=yes' '--with-x-toolkit=gtk' '--with-toolkit-scroll-bars' 'build_alias=i486-linux-gnu' 'host_alias=i486-linux-gnu' 'CFLAGS=-DDEBIAN -g -O2' 'LDFLAGS=-g' 'CPPFLAGS=''

Important settings:
  value of $LC_ALL: en_GB.UTF-8
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: en_GB.UTF-8
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_GB.UTF-8
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8-unix
  default-enable-multibyte-characters: t

Major mode: Org

Minor modes in effect:
  erc-list-mode: t
  erc-menu-mode: t
  erc-autojoin-mode: t
  erc-ring-mode: t
  erc-networks-mode: t
  erc-pcomplete-mode: t
  erc-track-mode: t
  erc-track-minor-mode: t
  erc-match-mode: t
  erc-button-mode: t
  erc-fill-mode: t
  erc-stamp-mode: t
  erc-netsplit-mode: t
  diff-auto-refine-mode: t
  display-time-mode: t
  display-battery-mode: t
  jabber-activity-mode: t
  jabber-mode-line-mode: t
  erc-irccontrols-mode: t
  erc-noncommands-mode: t
  erc-move-to-prompt-mode: t
  erc-readonly-mode: t
  show-paren-mode: t
  delete-selection-mode: t
  iswitchb-mode: t
  shell-dirtrack-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  global-auto-composition-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
Making thread...done
Updating marks...
Scoring...done
Updated (-0/+1) message(s).
Updated (-0/+1) message(s)
Retrieving...done
Disposing messages...done
ERC Version 5.3
Mark set
Making completion list...

bug#5650: Proposed fix

[Vivek Dasmohapatra]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 36 bytes --]

Does this patch fix things for you?

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: TEXT/x-diff; name=0001-Make-the-default-erc-connection-method-non-blocking.patch, Size: 2181 bytes --]

diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -362,7 +362,7 @@ alist."
   :type '(repeat (cons (string :tag "Target")
                        coding-system)))
 
-(defcustom erc-server-connect-function 'open-network-stream
+(defcustom erc-server-connect-function 'erc-open-network-stream
   "Function used to initiate a connection.
 It should take same arguments as `open-network-stream' does."
   :group 'erc-server
@@ -483,6 +483,10 @@ The current buffer is given by BUFFER."
        (memq (process-status erc-server-process) '(run open))))
 
 ;;;; Connecting to a server
+(defun erc-open-network-stream (name buffer host service)
+  "As `open-network-stream', but does non-blocking IO"
+  (make-network-process :name name :buffer  buffer
+                        :host host :service service :nowait t))
 
 (defun erc-server-connect (server port buffer)
   "Perform the connection and login using the specified SERVER and PORT.
@@ -523,9 +527,8 @@ We will store server variables in the buffer given by BUFFER."
             (with-current-buffer buffer (erc-current-nick))))
   ;; wait with script loading until we receive a confirmation (first
   ;; MOTD line)
-  (if (eq erc-server-connect-function 'open-network-stream-nowait)
-      ;; it's a bit unclear otherwise that it's attempting to establish a
-      ;; connection
+  (if (eq (process-status process) 'connect)
+      ;; waiting for anon-blocking connect - keep the user informed
       (erc-display-message nil nil buffer "Opening connection..\n")
     (erc-login)))
 
@@ -543,7 +546,7 @@ Make sure you are in an ERC buffer when running this."
       (setq erc-server-last-sent-time 0)
       (setq erc-server-lines-sent 0)
       (let ((erc-server-connect-function (or erc-session-connector
-                                             'open-network-stream)))
+                                             'erc-open-network-stream)))
         (erc-open erc-session-server erc-session-port erc-server-current-nick
                   erc-session-user-full-name t erc-session-password)))))
 

bug#5650: Oops, bad patch.

[Vivek Dasmohapatra]

The patch is a little broken - would have fixed it earlier but mu HD
exploded and I've been resurrecting my laptop from backups.

Will fix shortly.

bug#5650: updated patch

[Vivek Dasmohapatra]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 108 bytes --]

Updated patch, currently in use on my machine, byte compiles cleanly this 
time and appears to as expected.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: TEXT/x-diff; name=0001-Make-an-async-connection-in-attempt-by-default-in-er.patch, Size: 5431 bytes --]

diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -362,7 +362,7 @@ alist."
   :type '(repeat (cons (string :tag "Target")
                        coding-system)))
 
-(defcustom erc-server-connect-function 'open-network-stream
+(defcustom erc-server-connect-function 'erc-open-network-stream
   "Function used to initiate a connection.
 It should take same arguments as `open-network-stream' does."
   :group 'erc-server
@@ -483,51 +483,53 @@ The current buffer is given by BUFFER."
        (memq (process-status erc-server-process) '(run open))))
 
 ;;;; Connecting to a server
+(defun erc-open-network-stream (name buffer host service)
+  "As `open-network-stream', but does non-blocking IO"
+  (make-network-process :name name :buffer  buffer
+                        :host host :service service :nowait t))
 
 (defun erc-server-connect (server port buffer)
   "Perform the connection and login using the specified SERVER and PORT.
 We will store server variables in the buffer given by BUFFER."
-  (let ((msg (erc-format-message 'connect ?S server ?p port)))
+  (let ((msg (erc-format-message 'connect ?S server ?p port)) process)
     (message "%s" msg)
-    (let ((process (funcall erc-server-connect-function
-                            (format "erc-%s-%s" server port)
-                            nil server port)))
-      (unless (processp process)
-        (error "Connection attempt failed"))
+    (setq process (funcall erc-server-connect-function
+                           (format "erc-%s-%s" server port) nil server port))
+    (unless (processp process)
+      (error "Connection attempt failed"))
+    ;; Misc server variables
+    (with-current-buffer buffer
+      (setq erc-server-process process)
+      (setq erc-server-quitting nil)
+      (setq erc-server-reconnecting nil)
+      (setq erc-server-timed-out nil)
+      (setq erc-server-banned nil)
+      (setq erc-server-error-occurred nil)
+      (let ((time (erc-current-time)))
+        (setq erc-server-last-sent-time time)
+        (setq erc-server-last-ping-time time)
+        (setq erc-server-last-received-time time))
+      (setq erc-server-lines-sent 0)
+      ;; last peers (sender and receiver)
+      (setq erc-server-last-peers '(nil . nil)))
+    ;; we do our own encoding and decoding
+    (when (fboundp 'set-process-coding-system)
+      (set-process-coding-system process 'raw-text))
+    ;; process handlers
+    (set-process-sentinel process 'erc-process-sentinel)
+    (set-process-filter process 'erc-server-filter-function)
+    (set-process-buffer process buffer)
+    (erc-log "\n\n\n********************************************\n")
+    (message "%s" (erc-format-message
+                   'login ?n
+                   (with-current-buffer buffer (erc-current-nick))))
+    ;; wait with script loading until we receive a confirmation (first
+    ;; MOTD line)
+    (if (eq (process-status process) 'connect)
+        ;; waiting for a non-blocking connect - keep the user informed
+        (erc-display-message nil nil buffer "Opening connection..\n")
       (message "%s...done" msg)
-      ;; Misc server variables
-      (with-current-buffer buffer
-        (setq erc-server-process process)
-        (setq erc-server-quitting nil)
-        (setq erc-server-reconnecting nil)
-        (setq erc-server-timed-out nil)
-        (setq erc-server-banned nil)
-        (setq erc-server-error-occurred nil)
-        (let ((time (erc-current-time)))
-          (setq erc-server-last-sent-time time)
-          (setq erc-server-last-ping-time time)
-          (setq erc-server-last-received-time time))
-        (setq erc-server-lines-sent 0)
-        ;; last peers (sender and receiver)
-        (setq erc-server-last-peers '(nil . nil)))
-      ;; we do our own encoding and decoding
-      (when (fboundp 'set-process-coding-system)
-        (set-process-coding-system process 'raw-text))
-      ;; process handlers
-      (set-process-sentinel process 'erc-process-sentinel)
-      (set-process-filter process 'erc-server-filter-function)
-      (set-process-buffer process buffer)))
-  (erc-log "\n\n\n********************************************\n")
-  (message "%s" (erc-format-message
-            'login ?n
-            (with-current-buffer buffer (erc-current-nick))))
-  ;; wait with script loading until we receive a confirmation (first
-  ;; MOTD line)
-  (if (eq erc-server-connect-function 'open-network-stream-nowait)
-      ;; it's a bit unclear otherwise that it's attempting to establish a
-      ;; connection
-      (erc-display-message nil nil buffer "Opening connection..\n")
-    (erc-login)))
+      (erc-login)) ))
 
 (defun erc-server-reconnect ()
 "Reestablish the current IRC connection.
@@ -543,7 +545,7 @@ Make sure you are in an ERC buffer when running this."
       (setq erc-server-last-sent-time 0)
       (setq erc-server-lines-sent 0)
       (let ((erc-server-connect-function (or erc-session-connector
-                                             'open-network-stream)))
+                                             'erc-open-network-stream)))
         (erc-open erc-session-server erc-session-port erc-server-current-nick
                   erc-session-user-full-name t erc-session-password)))))
 

bug#5650: updated patch

[Lars Ingebrigtsen]

Vivek Dasmohapatra <vivek@etla.org> writes:

> Updated patch, currently in use on my machine, byte compiles cleanly
> this time and appears to as expected.

Thanks; applied to the Emacs trunk.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#5650: Issue with the patch

[Thomas Riccardi]

[-- Attachment #1: Type: text/plain, Size: 822 bytes --]

The patch works for me (even if there is still a small freeze during
about 1 second at the connection if the server doesn't answer), but
creates an issue with the auto reconnect feature.

It happens when the server doesn't answer the connection request (tested
with a DROP on packets coming from it with iptables).

The patch makes the function erc-server-connect to return even if the
connection is not ready. Then erc-open and erc-server-reconnect do the
same.

Finally, in erc-process-sentinel-2, there is no error executing
erc-server-reconnect, thus erc-server-reconnect-count is never
incremented and is even reset.

I made a patch that increments erc-server-reconnect-count before calling
erc-server-reconnect instead, and resets it only when the connection is
really ready, that is, in erc-connection-established.

[-- Attachment #2: Fix-reconnect-attempts-count.patch --]
[-- Type: text/x-patch, Size: 1630 bytes --]

diff --git a/.emacs.d/erc-5.3/erc-backend.el b/.emacs.d/erc-5.3/erc-backend.el
index 70ce789..928770e 100644
--- a/.emacs.d/erc-5.3/erc-backend.el
+++ b/.emacs.d/erc-5.3/erc-backend.el
@@ -618,13 +618,11 @@ EVENT is the message received from the closed connection process."
           (condition-case err
               (progn
                 (setq erc-server-reconnecting nil)
-                (erc-server-reconnect)
-                (setq erc-server-reconnect-count 0))
+                (setq erc-server-reconnect-count (1+ erc-server-reconnect-count))
+                (erc-server-reconnect))
             (error (when (buffer-live-p buffer)
                      (set-buffer buffer)
-                     (if (integerp erc-server-reconnect-attempts)
-                         (setq erc-server-reconnect-count
-                               (1+ erc-server-reconnect-count))
+                     (unless (integerp erc-server-reconnect-attempts)
                        (message "%s ... %s"
                                 "Reconnecting until we succeed"
                                 "kill the ERC server buffer to stop"))
diff --git a/.emacs.d/erc-5.3/erc.el b/.emacs.d/erc-5.3/erc.el
index 9aa5e8b..802ae66 100644
--- a/.emacs.d/erc-5.3/erc.el
+++ b/.emacs.d/erc-5.3/erc.el
@@ -4267,6 +4267,7 @@ Set user modes and run `erc-after-connect' hook."
 	    (nick (car (erc-response.command-args parsed)))
 	    (buffer (process-buffer proc)))
 	(setq erc-server-connected t)
+	(setq erc-server-reconnect-count 0)
 	(erc-update-mode-line)
 	(erc-set-initial-user-mode nick buffer)
 	(erc-server-setup-periodical-ping buffer)

bug#5650: Issue with the patch

[Lars Ingebrigtsen]

Thomas Riccardi <riccardi.thomas@gmail.com> writes:

> The patch works for me (even if there is still a small freeze during
> about 1 second at the connection if the server doesn't answer), but
> creates an issue with the auto reconnect feature.
>
> It happens when the server doesn't answer the connection request (tested
> with a DROP on packets coming from it with iptables).
>
> The patch makes the function erc-server-connect to return even if the
> connection is not ready. Then erc-open and erc-server-reconnect do the
> same.
>
> Finally, in erc-process-sentinel-2, there is no error executing
> erc-server-reconnect, thus erc-server-reconnect-count is never
> incremented and is even reset.
>
> I made a patch that increments erc-server-reconnect-count before calling
> erc-server-reconnect instead, and resets it only when the connection is
> really ready, that is, in erc-connection-established.
>

Thanks; applied to the Emacs trunk.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#5650: 23.1; ERC causes Emacs to hang on network dropouts

[Chong Yidong]

[-- Attachment #1: Type: text/plain, Size: 636 bytes --]

> > When my network connection drops out, ERC loses its connection and
> > attempts to reconnect. I see the message "connecting to server
> > irc.freenode.net 6777..." in the minibuffer and Emacs becomes
> > unresponsive.
> ...
> I made a patch that increments erc-server-reconnect-count before calling
> erc-server-reconnect instead, and resets it only when the connection is
> really ready, that is, in erc-connection-established.

Hi,

Could someone please review this patch by Thomas Riccardi and Vivek
Dasmohapatra?  The full description of the problem can be viewed at

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=5650

Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Fix-reconnect-attempts-count.patch --]
[-- Type: text/x-diff, Size: 1630 bytes --]

diff --git a/.emacs.d/erc-5.3/erc-backend.el b/.emacs.d/erc-5.3/erc-backend.el
index 70ce789..928770e 100644
--- a/.emacs.d/erc-5.3/erc-backend.el
+++ b/.emacs.d/erc-5.3/erc-backend.el
@@ -618,13 +618,11 @@ EVENT is the message received from the closed connection process."
           (condition-case err
               (progn
                 (setq erc-server-reconnecting nil)
-                (erc-server-reconnect)
-                (setq erc-server-reconnect-count 0))
+                (setq erc-server-reconnect-count (1+ erc-server-reconnect-count))
+                (erc-server-reconnect))
             (error (when (buffer-live-p buffer)
                      (set-buffer buffer)
-                     (if (integerp erc-server-reconnect-attempts)
-                         (setq erc-server-reconnect-count
-                               (1+ erc-server-reconnect-count))
+                     (unless (integerp erc-server-reconnect-attempts)
                        (message "%s ... %s"
                                 "Reconnecting until we succeed"
                                 "kill the ERC server buffer to stop"))
diff --git a/.emacs.d/erc-5.3/erc.el b/.emacs.d/erc-5.3/erc.el
index 9aa5e8b..802ae66 100644
--- a/.emacs.d/erc-5.3/erc.el
+++ b/.emacs.d/erc-5.3/erc.el
@@ -4267,6 +4267,7 @@ Set user modes and run `erc-after-connect' hook."
 	    (nick (car (erc-response.command-args parsed)))
 	    (buffer (process-buffer proc)))
 	(setq erc-server-connected t)
+	(setq erc-server-reconnect-count 0)
 	(erc-update-mode-line)
 	(erc-set-initial-user-mode nick buffer)
 	(erc-server-setup-periodical-ping buffer)

bug#5650: Status

[Michael McCracken]

[-- Attachment #1: Type: text/plain, Size: 230 bytes --]

Hi, I have been experiencing this issue, and am currently testing out the
patches in this thread. I was wondering if there's anything I can do to
help move this along, as it's a big problem for anyone with a flaky IRC
connection.

[-- Attachment #2: Type: text/html, Size: 262 bytes --]

bug#5650: 23.1; ERC causes Emacs to hang on network dropouts

[Lars Ingebrigtsen]

Chong Yidong <cyd@stupidchicken.com> writes:

>> > When my network connection drops out, ERC loses its connection and
>> > attempts to reconnect. I see the message "connecting to server
>> > irc.freenode.net 6777..." in the minibuffer and Emacs becomes
>> > unresponsive.
>> ...
>> I made a patch that increments erc-server-reconnect-count before calling
>> erc-server-reconnect instead, and resets it only when the connection is
>> really ready, that is, in erc-connection-established.
>
> Hi,
>
> Could someone please review this patch by Thomas Riccardi and Vivek
> Dasmohapatra?  The full description of the problem can be viewed at

The patches looked OK to me.  I've applied them to the Emacs trunk
now -- I think we can consider this "new functionality" and not a bug
fix.  It seems to work for me, but I guess time will tell..

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no