Issue 30394: smtplib leaves open sockets around if SMTPConnectError is raised in init (original) (raw)
Created on 2017-05-17 23:25 by jhillacre, last changed 2022-04-11 14:58 by admin. This issue is now closed.
Messages (10)
Author: Joel Hillacre (jhillacre) *
Date: 2017-05-17 23:25
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 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.
Author: Xiang Zhang (xiang.zhang) * 
Date: 2017-05-21 14:34
Joel, our patch system has moved to GitHub. Mind to turn your patch into a PR?
Author: Joel Hillacre (jhillacre) *
Date: 2017-05-21 20:41
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.
Author: R. David Murray (r.david.murray) *
Date: 2017-05-22 21:47
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 .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.
Author: Joel Hillacre (jhillacre) *
Date: 2017-05-23 16:47
r.david.murray,
How would a test would have a reference to after an exception in the smtplib.SMTP.init()?
Author: R. David Murray (r.david.murray) *
Date: 2017-05-23 21:11
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 :)
Author: Xiang Zhang (xiang.zhang) *
Date: 2017-05-24 05:14
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
Author: Xiang Zhang (xiang.zhang) *
Date: 2017-05-24 18:24
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
Author: Xiang Zhang (xiang.zhang) *
Date: 2017-05-24 18:49
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
Author: Xiang Zhang (xiang.zhang) *
Date: 2017-05-24 18:59
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
History
Date
User
Action
Args
2022-04-11 14:58:46
admin
set
github: 74579
2017-05-24 19:04:12
xiang.zhang
set
status: open -> closed
resolution: fixed
stage: patch review -> resolved
2017-05-24 18:59:08
xiang.zhang
set
messages: +
2017-05-24 18:49:26
xiang.zhang
set
messages: +
2017-05-24 18:24:29
xiang.zhang
set
messages: +
2017-05-24 17:13:57
matrixise
set
pull_requests: + <pull%5Frequest1872>
2017-05-24 17:13:45
matrixise
set
pull_requests: + <pull%5Frequest1871>
2017-05-24 17:13:15
matrixise
set
pull_requests: + <pull%5Frequest1870>
2017-05-24 05:14:53
xiang.zhang
set
messages: +
2017-05-23 21:11:17
r.david.murray
set
messages: +
2017-05-23 16:47:27
jhillacre
set
messages: +
2017-05-22 21:47:39
r.david.murray
set
nosy: + r.david.murray
messages: +
2017-05-21 20:41:09
jhillacre
set
messages: +
2017-05-21 20:32:56
jhillacre
set
pull_requests: + <pull%5Frequest1796>
2017-05-21 14:34:42
xiang.zhang
set
versions: + Python 2.7, Python 3.5, Python 3.7
nosy: + xiang.zhang
messages: +
type: enhancement -> behavior
stage: patch review
2017-05-17 23:25:17
jhillacre
set
files: + test_client.py
2017-05-17 23:25:12
jhillacre
set
files: + test_server.py
2017-05-17 23:25:01
jhillacre
create