classification
Title: socket.close() doesn't play well with __del__
Type: behavior Stage: resolved
Components: Extension Modules Versions: Python 3.2, Python 2.7
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: stutzbach Nosy List: BreamoreBoy, ajaksu2, isterika, jbrouwers, perrygreenfield, pitrou, stutzbach
Priority: normal Keywords: patch

Created on 2003-09-17 21:16 by perrygreenfield, last changed 2010-08-31 21:21 by stutzbach. This issue is now closed.

Files
File name Uploaded Description Edit
issue808164-27.patch stutzbach, 2010-08-18 21:40
issue808164-py3k.patch stutzbach, 2010-08-18 21:46
Messages (13)
msg60385 - (view) Author: Perry Greenfield (perrygreenfield) Date: 2003-09-17 21:16
It appears that 2.3 changed how socket.close() works. 
We now find errors when terminating Python when we 
reference socket.close() in a __del__ method. We have 
worked around it by removing calls to socket.close() but 
note the following differences between the same section 
of code between 2.2 and 2.3 which suggest that the 2.3 
behavior is not ideal and should be fixed.

In Python 2.3:

    def close(self):
        self._sock = _closedsocket()
        self.send = self.recv = self.sendto = self.recvfrom \
                        = self._sock._dummy

In Python 2.2:

    def close(self):
        # Avoid referencing globals here
        self._sock = self.__class__._closedsocket()

Note the reference to avoiding globals which is 
presumably the source of the problem with 2.3. Perhaps 
I'm naive but I would guess that calling a close method 
should be considered a safe operation for use in a 
__del__ method.
msg60386 - (view) Author: Jean M. Brouwers (jbrouwers) Date: 2004-02-25 20:38
Logged In: YES 
user_id=832557

Two other comments on socket.close() in Python 2.3.

In addition, the socket.close() method in the Lib/socket.py
class does not even call the _realsocket.close() method.

Before zapping the _sock attribute, shouldn't the latter
close() method be called first?  If not, what happens to
any data which may be buffered by the _realsocket?


Also, every time the socket.close() method is called
another, new instance of the _closedsocket class is
created.  That seems unnecessary waste.

Why not use a single instance created once at the
module level?
msg82029 - (view) Author: Daniel Diniz (ajaksu2) (Python triager) Date: 2009-02-14 12:22
Current code is:

    def close(self):
        self._sock = _closedsocket()
        dummy = self._sock._dummy
        for method in _delegate_methods:
            setattr(self, method, dummy)
    close.__doc__ = _realsocket.close.__doc__

It sure seems to be trying to avoid __del__ issues, does it fix this bug?
msg84883 - (view) Author: Daniel Stutzbach (stutzbach) (Python committer) Date: 2009-03-31 19:49
Daniel,

_closedsocket is a global, so it isn't safe to be used within __del__
during shutdown.

In the py3k branch, socket.__del__ calls socket._real_close which
references the global _socket.  So it's not safe their, either.
msg87961 - (view) Author: 77cc33@gmail.com (isterika) Date: 2009-05-17 06:18
The same is happened when you are trying to close pycurl handler at
__del__ method.
msg104383 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-04-27 23:44
Well, realistically, if you have open sockets at shutdown, there isn't a big difference between closing and not closing them. They will be reaped with the process anyway.
msg104385 - (view) Author: Daniel Stutzbach (stutzbach) (Python committer) Date: 2010-04-28 00:09
The purpose of calling .close() in __del__ is to close the socket when the owner is destroyed yet the program is still running.

This prevents the socket from staying open if some other part of the program has somehow acquired a reference to it.  telnetlib, urllib, and even the socket module itself contain classes that call .close() within __del__ for this purpose.

Unfortunately, that can result in ugly error messages when the interpreter is exiting.
msg104421 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-04-28 10:10
> Unfortunately, that can result in ugly error messages when the
> interpreter is exiting.

The classical solution is to early bind the necessary globals to
argument defaults, such as:

    def __del__(self, _socketclose=_socketclose):
        _socketclose(self._socket)
msg104541 - (view) Author: Daniel Stutzbach (stutzbach) (Python committer) Date: 2010-04-29 17:26
How do I write a unit test to check that socket.__del__ doesn't refer to any globals?
msg114272 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-08-18 19:48
Can someone reply to Daniel Stutzbach's query in msg104541, thanks.
msg114281 - (view) Author: Daniel Stutzbach (stutzbach) (Python committer) Date: 2010-08-18 21:39
I suspect that it's not possible.

I'm uploading patches that fix the .close method to avoid referencing globals that might be None during interpreter shutdown.  Since the .close method has changed significantly in py3k, I'm uploading separate patches for py3k and 2.7.
msg114282 - (view) Author: Daniel Stutzbach (stutzbach) (Python committer) Date: 2010-08-18 21:44
Here is the py3k version of the patch
msg115277 - (view) Author: Daniel Stutzbach (stutzbach) (Python committer) Date: 2010-08-31 21:21
Fixed in r84380 and r84382.
History
Date User Action Args
2010-08-31 21:21:22stutzbachsetstatus: open -> closed
messages: + msg115277

keywords: - needs review
resolution: accepted
stage: patch review -> resolved
2010-08-18 21:46:38stutzbachsetfiles: + issue808164-py3k.patch
2010-08-18 21:44:37stutzbachsetmessages: + msg114282
2010-08-18 21:40:15stutzbachsetkeywords: + patch, needs review
files: + issue808164-27.patch
stage: test needed -> patch review
2010-08-18 21:39:18stutzbachsetmessages: + msg114281
2010-08-18 19:48:55BreamoreBoysetnosy: + BreamoreBoy
messages: + msg114272
2010-04-29 17:26:36stutzbachsetmessages: + msg104541
2010-04-28 10:10:42pitrousetmessages: + msg104421
2010-04-28 00:09:28stutzbachsetmessages: + msg104385
2010-04-27 23:44:13pitrousetnosy: + pitrou
messages: + msg104383
2010-04-27 15:35:18stutzbachsetassignee: stutzbach
versions: + Python 2.7, Python 3.2, - Python 2.5
2009-05-17 06:18:26isterikasetnosy: + isterika

messages: + msg87961
versions: + Python 2.5, - Python 2.6, Python 3.0, Python 3.1, Python 2.7
2009-03-31 19:49:13stutzbachsetnosy: + stutzbach

messages: + msg84883
versions: + Python 3.0, Python 3.1, Python 2.7
2009-02-14 12:22:28ajaksu2setnosy: + ajaksu2
messages: + msg82029
stage: test needed
2008-01-20 19:11:36christian.heimessettype: behavior
components: + Extension Modules, - Library (Lib)
versions: + Python 2.6, - Python 2.3
2003-09-17 21:16:55perrygreenfieldcreate