[issue30394] smtplib leaves open sockets around if SMTPConnectError is raised in __init__

[Xiang Zhang]

Changes by Xiang Zhang :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30394] smtplib leaves open sockets around if SMTPConnectError is raised in __init__

[Xiang Zhang]

Xiang Zhang added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30394] smtplib leaves open sockets around if SMTPConnectError is raised in __init__

[Xiang Zhang]

Xiang Zhang added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30394] smtplib leaves open sockets around if SMTPConnectError is raised in __init__

[Xiang Zhang]

Xiang Zhang added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30394] smtplib leaves open sockets around if SMTPConnectError is raised in __init__

[Stéphane Wirtel]

Changes by Stéphane Wirtel :


--
pull_requests: +1871

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30394] smtplib leaves open sockets around if SMTPConnectError is raised in __init__

[Stéphane Wirtel]

Changes by Stéphane Wirtel :


--
pull_requests: +1870

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30394] smtplib leaves open sockets around if SMTPConnectError is raised in __init__

[Stéphane Wirtel]

Changes by Stéphane Wirtel :


--
pull_requests: +1872

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30394] smtplib leaves open sockets around if SMTPConnectError is raised in __init__

[Xiang Zhang]

Xiang Zhang added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30394] smtplib leaves open sockets around if SMTPConnectError is raised in __init__

[R. David Murray]

R. David Murray added the comment:

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 :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30394] smtplib leaves open sockets around if SMTPConnectError is raised in __init__

[Joel Hillacre]

Joel Hillacre added the comment:

r.david.murray,

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30394] smtplib leaves open sockets around if SMTPConnectError is raised in __init__

[R. David Murray]

R. David Murray added the comment:

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.

--
nosy: +r.david.murray

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30394] smtplib leaves open sockets around if SMTPConnectError is raised in __init__

[Joel Hillacre]

Joel Hillacre added the comment:

> 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30394] smtplib leaves open sockets around if SMTPConnectError is raised in __init__

[Joel Hillacre]

Changes by Joel Hillacre :


--
pull_requests: +1796

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30394] smtplib leaves open sockets around if SMTPConnectError is raised in __init__

[Xiang Zhang]

Xiang Zhang added the comment:

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

--
nosy: +xiang.zhang
stage:  -> patch review
type: enhancement -> behavior
versions: +Python 2.7, Python 3.5, Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30394] smtplib leaves open sockets around if SMTPConnectError is raised in __init__

[Joel Hillacre]

Changes by Joel Hillacre :


Added file: http://bugs.python.org/file46875/test_client.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30394] smtplib leaves open sockets around if SMTPConnectError is raised in __init__

[Joel Hillacre]

Changes by Joel Hillacre :


Added file: http://bugs.python.org/file46874/test_server.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30394] smtplib leaves open sockets around if SMTPConnectError is raised in __init__

[Joel Hillacre]

New submission from Joel Hillacre:

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 
$ 


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.

--
components: Library (Lib)
files: smtplib.patch
keywords: patch
messages: 293905
nosy: jhillacre
priority: normal
severity: normal
status: open
title: smtplib leaves open sockets around if SMTPConnectError is raised in 
__init__
type: enhancement
versions: Python 3.6
Added file: http://bugs.python.org/file46873/smtplib.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com