When running "make test" on Python3, test_socket reports a number of ResourceWarnings due to unclosed sockets.  Attached is a patch that changes the relevant tests so that they close all the created sockets.

test_multiprocessing and test_xmlrpc have a similar problem; I will upload patches for these shortly.

Attached is a patch that fixes the warnings in test_xmlrpc, along with some other file- and socket-related warnings in test_normalization, test_timeout and test_tk that only show up when regrtest is run with -uall.

The warning in test_timeout could be fixed with a smaller modification of the test code, but I thought it was better to have two separate attributes for the two sockets.  It seemed misleading to have _some_ of the setup/teardown code in setUp() and tearDown(), but then be doing more in the actual tests.

The warnings in test_multiprocessing seem to be due to leaks in the actual multiprocessing module, not in the test code, so that might be a bit more work to fix.

test_cgi causes a strange filehandle leak that only causes a warning when regrtest terminates, and for some reason doesn't show up if you run just test_cgi by itself.  I've attached a patch that closes the filehandle.

* r87680 fixes test_sockserver
 * r87681 fixes test_timeout
 * r87682 fixes test_tk
 * r87683 fixes test_xmlrpc
 * r87684 fixes test_socket

r87682, r87683, r87684 are patches from Nadeem Vawda.

On my Linux box, I am unable to get the warning on test_cgi or test_normalization.

* r87686 fixes multiprocessing
 * r87687 fixes pydoc
 * r87688 fixes test_subprocess

Remaining ResourceWarning warnings:
 * test_imaplib
 * test_urllibnet
 * test_urllib2net

The fix for test_normalization was committed as r87441. As for test_cgi, I still seem to get the leak (also on Linux; Ubuntu 10.10 64-bit). I'll poke around with it some more tomorrow.

In addition to the ResourceWarnings, some of tests have been raising DeprecationWarnings:
* test_unittest
* test_array
* test_httplib (trivial fix - replace assertEquals with assertEqual)

DeprecationWarnings:
* test_unittest: fixed by r87717
* test_array: fixed by r87719
* test_httplib: fixed by r87720

r87710 introduces a ResourceWarning in test_threading. Fix attached.

r87736 introduces another DeprecationError; this time in test_time (line 150; s/assertEquals/assertEqual/).

> r87710 introduces a ResourceWarning in test_threading. Fix attached.

Fixed by r87757 (I wrote a different patch).

> r87736 introduces another DeprecationError; this time in test_time (line 150; s/assertEquals/assertEqual/).

Fixed by r87759.

Fix attached for test_imaplib. Most of the warnings were simply due to reap_server() not closing the server object correctly. The remaining warning was due a genuine leak in imaplib.IMAP4.__init__() - if an exception is raised after the connection is opened, the socket is not closed.

> Fix attached for test_imaplib

Oh thanks! Commited as r87777 and r87778. I just changed the name of the subfunction.

Awesome. That just leaves test_urllibnet, test_urllib2net, and test_cgi. I'm hoping to post patches for the first two tomorrow.

About test_cgi, I've fiddled around with it a bit more. The leak manifests itself with any set of tests including test_cgi and test___all__, for example:

  ☿ ./python -Wd -E -bb -m test.regrtest test___all__ test_cgi
  [1/2] test___all__
  [2/2] test_cgi
  All 2 tests OK.
  sys:1: ResourceWarning: unclosed file <_io.TextIOWrapper name='/dev/null' encoding='UTF-8'>

... but not with any other 2-test combination. This led me to think it was something specific to test___all__, but it does also come up when running all tests *except* test___all__.

I'm guessing there's something somewhere that's causing the cgi module to be garbage-collected between the tests finishing and the process terminating. Without some familiarity with unittest's internals, I can't say anything more, though.

test___all__ just loads ALL modules... and it doesn't unload them. I patched test___all__ to unload modules: the "ResourceWarning: unclosed file ... '/dev/null' ..." disappears, but a new error occurs. The multiprocessing module registers _exit_function() in the atexit module. If this module is unloaded, the callback stays in atexit, but the callback will raise an error when it is called because all module variables were cleared (set to None)...

Have you tried my patch (resourcewarning-fixes-3.diff)? It fixes the warning for me without breaking anything.

I was just worried that the warning was something triggered by my specific system configuration when you said that you couldn't reproduce it. I was trying to see if I could find a reason why it might appear on one system but not another. If you *have* been able to reproduce it, then I needn't look any further :P

Looking at the warnings from test_urllib2net, it seems that they all originate in the FTP tests:
* OtherNetworkTests.test_ftp()
* TimeoutTest.test_ftp_basic()
* TimeoutTest.test_ftp_default_timeout()

Most of these leaks seem to stem from the fact that socket.SocketIO.close() doesn't behave as documented. According to its docstring, it is meant to decrement the underlying socket's refcount, and close it if the refcount drops to zero. However, to do this job it calls socket._decref_socketios(), which is defined as follows:

    def _decref_socketios(self):
        if self._io_refs > 0:
            self._io_refs -= 1
        if self._closed:
            self.close()

Clearly, this doesn't do what the docstring describes. Changing the second conditional from "if self._closed:" to "if self._io_refs <= 0:" disposes of all but one of the ResourceWarnings, but also breaks 8 tests in test_socket. It seems that the tests expect a socket to remain open after all referring SocketIO objects have been closed, which contradicts the docstring for SocketIO.close(). I suppose I should open a separate issue for this.


The remaining warning occurs in test_ftp() when retrieving a non-existent file; I haven't yet managed to figure out what is causing it.

Le vendredi 07 janvier 2011 à 02:19 +0000, Nadeem Vawda a écrit :
> Most of these leaks seem to stem from the fact that socket.SocketIO.close() doesn't behave as documented. According to its docstring, it is meant to decrement the underlying socket's refcount, and close it if the refcount drops to zero. However, to do this job it calls socket._decref_socketios(), which is defined as follows:
> 
>     def _decref_socketios(self):
>         if self._io_refs > 0:
>             self._io_refs -= 1
>         if self._closed:
>             self.close()
> 
> Clearly, this doesn't do what the docstring describes. Changing the second conditional from "if self._closed:" to "if self._io_refs <= 0:" disposes of all but one of the ResourceWarnings, but also breaks 8 tests in test_socket. It seems that the tests expect a socket to remain open after all referring SocketIO objects have been closed, which contradicts the docstring for SocketIO.close(). I suppose I should open a separate issue for this.

Can you please open a new issue for that?

Victor

Sorry, scratch that - I misunderstood the semantics of SocketIO.close(). I hadn't realized that the underlying socket is supposed to stay open until it itself is also explicitly closed (as well as all SocketIO objects referring to it).

I've been able to get rid of 2 of the 7 warnings in test_urllib2net with the following change:

  diff --git a/Lib/urllib/request.py b/Lib/urllib/request.py
  --- a/Lib/urllib/request.py
  +++ b/Lib/urllib/request.py
  @@ -2151,7 +2151,9 @@
               conn = self.ftp.ntransfercmd(cmd)
           self.busy = 1
           # Pass back both a suitably decorated object and a retrieval length
  -        return (addclosehook(conn[0].makefile('rb'), self.endtransfer), conn[1])
  +        fp = addclosehook(conn[0].makefile('rb'), self.endtransfer)
  +        conn[0].close()
  +        return (fp, conn[1])
       def endtransfer(self):
           if not self.busy:
               return

It seems that most of the remaining warnings are the result of FTPHandler.ftp_open() not doing anything to close the ftpwrapper objects it creates. I haven't been able to figure out exactly what the correct place to do this is, though.

I opened a separated issue for test_urllib and test_urllib2net: #10883.

Good idea; they look like more work to fix than the warnings so far. Aside from those two, it looks like test_cgi is all that's left. Just to clarify, did you manage to reproduce the test_cgi warning?

Attached is a simpler fix for test_cgi so it can get in for Python 3.2. You can reproduce the failure if you run ``./python -W error -m test test_cgi``.

Georg, can I commit?

Looks good to me. My earlier patch was more defensive because I wasn't sure whether any of the other tests might be using cgi.log(), but it seems that this isn't the case.

Once Python 3.3 is open I will apply the cgi fix. Just to double-check, can I close this issue once the test_cgi patch goes in?

No objections here. The other remaining leaks that I am aware of are covered by issue10883.

r88496 for 3.3
r88497 for 3.2