Patchwork dummysmtpd: don't die on client connection errors

login
register
mail settings
Submitter Matt Harbison
Date Jan. 22, 2018, 6:57 a.m.
Message ID <915dd5fcdb12a6e2f764.1516604225@Envy>
Download mbox | patch
Permalink /patch/27028/
State Accepted
Headers show

Comments

Matt Harbison - Jan. 22, 2018, 6:57 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1516599582 18000
#      Mon Jan 22 00:39:42 2018 -0500
# Node ID 915dd5fcdb12a6e2f7644f8825fcab849245d738
# Parent  0da50dc8ce0f4ccfe5acabbe5880d1fd7993282c
dummysmtpd: don't die on client connection errors

The connection refused error in test-patchbomb-tls.t[1] is sporadic, but one of
the more often seen errors on Windows.  I added enough logging to a file and
dumped it out at the end to make the following observations:

  - The listening socket is successfully created and bound to the port, and the
    "listening at..." message is always logged.

  - Generally, the following is the entire log output, with the "accepted ..."
    message having been added after `sslutil.wrapserversocket`:

          listening at localhost:$HGPORT
          $LOCALIP ssl error
          accepted connect
          accepted connect
          $LOCALIP from=quux to=foo, bar
          $LOCALIP ssl error

  - In the cases that fail, asyncore.loop() in the run() method is exiting, but
    not with an exception.

  - In the cases that fail, the following is logged right after "listening ...":

        Traceback (most recent call last):
          File "c:\\Python27\\lib\\asyncore.py", line 83, in read
            obj.handle_read_event()
          File "c:\\Python27\\lib\\asyncore.py", line 443, in handle_read_event
            self.handle_accept()
          File "../tests/dummysmtpd.py", line 80, in handle_accept
            conn = sslutil.wrapserversocket(conn, ui, certfile=self._certfile)
          File "..\\mercurial\\sslutil.py", line 570, in wrapserversocket
            return sslcontext.wrap_socket(sock, server_side=True)
          File "c:\\Python27\\lib\\ssl.py", line 363, in wrap_socket
            _context=self)
          File "c:\\Python27\\lib\\ssl.py", line 611, in __init__
            self.do_handshake()
          File "c:\\Python27\\lib\\ssl.py", line 840, in do_handshake
            self._sslobj.do_handshake()
        error: [Errno 10054] $ECONNRESET$

  - If the base class handler is overridden completely, the the first "ssl
    error" line is replaced by the stacktrace, but the other lines are
    unchanged.  The client behaves no differently, whether or not the server
    stacktraced.

In general, `./run-tests.py --local -j9 -t9000 test-patchbomb-tls.t
--runs-per-test 20` would show the issue after a run or two.  With this change,
`./run-tests.py --local -j9 -t9000 test-patchbomb-tls.t --loop` ran 800 times
without a hiccup.  This makes me wonder if the other connection refused messages
that bubble up on occasion are caused by a similar issue.  It seems a bit
drastic to kill the whole server on account of a single communication failure
with a client.

# no-check-commit because of handle_error()

[1] https://buildbot.mercurial-scm.org/builders/Win7%20x86_64%20hg%20tests/builds/421/steps/run-tests.py%20%28python%202.7.13%29/logs/stdio
Yuya Nishihara - Jan. 22, 2018, 12:09 p.m.
On Mon, 22 Jan 2018 01:57:05 -0500, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1516599582 18000
> #      Mon Jan 22 00:39:42 2018 -0500
> # Node ID 915dd5fcdb12a6e2f7644f8825fcab849245d738
> # Parent  0da50dc8ce0f4ccfe5acabbe5880d1fd7993282c
> dummysmtpd: don't die on client connection errors

Seems fine. Queued, thanks.

Patch

diff --git a/tests/dummysmtpd.py b/tests/dummysmtpd.py
--- a/tests/dummysmtpd.py
+++ b/tests/dummysmtpd.py
@@ -9,6 +9,7 @@ 
 import smtpd
 import ssl
 import sys
+import traceback
 
 from mercurial import (
     server,
@@ -27,6 +28,15 @@ 
     def process_message(self, peer, mailfrom, rcpttos, data):
         log('%s from=%s to=%s\n' % (peer[0], mailfrom, ', '.join(rcpttos)))
 
+    def handle_error(self):
+        # On Windows, a bad SSL connection sometimes generates a WSAECONNRESET.
+        # The default handler will shutdown this server, and then both the
+        # current connection and subsequent ones fail on the client side with
+        # "No connection could be made because the target machine actively
+        # refused it".  If we eat the error, then the client properly aborts in
+        # the expected way, and the server is available for subsequent requests.
+        traceback.print_exc()
+
 class dummysmtpsecureserver(dummysmtpserver):
     def __init__(self, localaddr, certfile):
         dummysmtpserver.__init__(self, localaddr)