I am encountering a ResourceWarning about an unclosed socket when getting a non 220 response during connect() in __init__() of smtplib.SMTP. Attached are a client script causing this warning for me, a server script to cause the client to the warning and a patch that fixes the warning for me. My python version is Python 3.6.1 (default, Apr  7 2017, 09:32:32) [GCC 4.8.5 20150623 (Red Hat 4.8.5-11)] on linux. I had found previous related issue with similar symptom and remedy in issue #21641.


$ python3.6 test_server.py
connected by ('127.0.0.1', 53630)
$ 


$ python3.6 test_client.py
Traceback (most recent call last):
  File "test_client.py", line 2, in <module>
    smtp = smtplib.SMTP('127.0.0.1', 8025)
  File "/usr/lib64/python3.6/smtplib.py", line 253, in __init__
    raise SMTPConnectError(code, msg)
smtplib.SMTPConnectError: (554, b'Nope.')
/usr/lib64/python3.6/socket.py:657: ResourceWarning: unclosed <socket.socket fd=4, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=6, laddr=('127.0.0.1', 53630), raddr=('127.0.0.1', 8025)>
$ 


RFC 2821 states that servers responding with non 220 greetings must not close the connection. It is this behaviour that test_server.py is using to trigger the warning in test_client.py.

RFC 2821 Section 3.1 Paragraph 3
https://tools.ietf.org/html/rfc2821#section-3.1
   '... a 554 response MAY be given in the initial connection opening message
   instead of the 220.  A server taking this approach MUST still wait
   for the client to send a QUIT (see section 4.1.1.10) before closing
   the connection and SHOULD respond to any intervening commands with
   "503 bad sequence of commands".'

The ResourceWarning is no longer caused for me after applying this change.

Joel, our patch system has moved to GitHub. Mind to turn your patch into a PR?

> Joel, our patch system has moved to GitHub. Mind to turn your patch into a PR?

I have opened a PR now.

I took a look at changing my example into a test. I am not sure how to test for a lack of warning. Closest test I found was BadHELOServerTests in test_smtplib.py, but using mock_socket would not cause the warning from socket.py.

It would be a white-box test, which I don't like, but it might be worth it to write a test that would check that <smtpobj>.sock is None, indicating that close was called.  You really can't check for no warning because when the warning gets generated is effectively asynchronous.

r.david.murray,

How would a test would have a reference to <smtpobj> after an exception in the smtplib.SMTP.__init__()?

Duh.  (Smacks self on forehead).

Nevermind.

I'll approve the patch as is, since I can't see any good way to test it.

(I suppose that we could factor the __init__ method contents out into something we could test, but I'm not going to push for that kind of refactoring for this simple change.  It probably comes down to connect being called in init being a design bug :)

New changeset 9e98cd0383d9e7f06c0537038a32459bf5efa97a by Xiang Zhang (Joel Hillacre) in branch 'master':
bpo-30394: Fix a socket leak in smtplib.SMTP.__init__() (#1700)
https://github.com/python/cpython/commit/9e98cd0383d9e7f06c0537038a32459bf5efa97a

New changeset 779e7c933e777270897b1e35fa9e5b12eee12af9 by Xiang Zhang (Stéphane Wirtel) in branch '2.7':
bpo-30394: Fix a socket leak in smtplib.SMTP.__init__() (#1700) (#1788)
https://github.com/python/cpython/commit/779e7c933e777270897b1e35fa9e5b12eee12af9

New changeset ebbefae14039aa86d4c8a7cfab8f2b5a3ef0d241 by Xiang Zhang (Stéphane Wirtel) in branch '3.5':
bpo-30394: Fix a socket leak in smtplib.SMTP.__init__() (#1700) (#1789)
https://github.com/python/cpython/commit/ebbefae14039aa86d4c8a7cfab8f2b5a3ef0d241

New changeset c3454f0e79b35fb81b0426cfac4b801d4495b8ea by Xiang Zhang (Stéphane Wirtel) in branch '3.6':
bpo-30394: Fix a socket leak in smtplib.SMTP.__init__() (#1700) (#1790)
https://github.com/python/cpython/commit/c3454f0e79b35fb81b0426cfac4b801d4495b8ea