This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: test_support.find_unused_port can cause socket conflicts on Windows
Type: behavior Stage:
Components: Tests, Windows Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: exarkun, giampaolo.rodola, iritkatriel, pitrou, r.david.murray, tim.golden, trent
Priority: normal Keywords: buildbot, patch

Created on 2010-04-30 09:18 by paul.moore, last changed 2022-04-11 14:57 by admin.

Files
File name Uploaded Description Edit
unused1.patch paul.moore, 2010-04-30 16:30 review
test_logging.patch paul.moore, 2010-05-01 10:26 review
Messages (23)
msg104614 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2010-04-30 09:18
test_support.find_unused_port attempts to find an unused port to use. The approach is fragile (as noted in the docstring) in certain cases. In particular, it appears that Windows takes a short time to free the socket after it is closed, meaning that when the caller tries to open a socket on the returned port number, it gets permission issues.

A sleep(0.1) just before returning the port number appears to address the issue, but this is not a proper solution.

http://msdn.microsoft.com/en-us/library/ms737582%28v=VS.85%29.aspx describes the SO_LINGER and SO_DONTLINGER socket options, which may be related, but in my initial tests I haven't been able to get these to work.
msg104638 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2010-04-30 14:18
find_unused_port is the wrong approach altogether.  Uses of it should be replaced by bind_port and then find_unused_port should be removed.
msg104651 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2010-04-30 16:30
OK. I've attached a patch which removes the use of get_unused_port for test_smtplib and test_multiprocessing. It doesn't use bind_port as both cases test classes that create their own port internally, rather I've used port 0 and then introspected the actual port assigned.

The same process could probably be used for the remaining uses of get_unused_port, but I haven't tackled them yet.

Tested on my local PC and on the buildbot where I see the bug. Both tests run fine in both cases. I'm running the full test suite now.
msg104677 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-04-30 23:26
> OK. I've attached a patch which removes the use of get_unused_port for 
> test_smtplib and test_multiprocessing.

Great, thank you. It was committed in r80669 (trunk), r80670 (2.6), r80671 (py3k), r80672 (3.1).
Note that there are still a couple of tests using find_unused_port, depending on the branch: test_httplib, test_logging, test_socket.
msg104701 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2010-05-01 10:26
Here's a patch for test_logging. It needed a minor tweak to logging.config - but I can't see anywhere that this affects the documentation, so I didn't do a doc patch. I hope that's OK.

I'll have a look at test_socket but that looks a bit too deep for me. And test_httplib seems to be testing that if you supply a specific port, the connection socket actually connects on that port, so I can't really see any way of avoiding needing a specific unused port there :-(
msg104710 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-05-01 11:08
Vinay, are you ok with the proposed patch?
msg104715 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-05-01 13:53
Could you special case the test_socket test by checking for the error that Windows sometimes throws and retrying (in a loop for say a second)?  Not ideal, but probably better than adding a sleep or throwing away the test :)
msg104724 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2010-05-01 15:15
Might work - but the only ones that were actually failing for me were test_multiprocessing and test_smtplib. So I'm not quite sure where/when the error would be raised on the remaining 2 (socket & httplib). But I'll keep it in mind.

To be honest, the remaining tests haven't actually failed in my experience, I'm now really only seeing if I can remove all uses of find_unused_port on the basis of Jean-Paul's comment that it's "the wrong approach".
msg104806 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-05-02 21:38
> find_unused_port is the wrong approach altogether.  Uses of it should 
> be replaced by bind_port and then find_unused_port should be removed

I agree find_unused_port() is the wrong approach in general, but there are certain cases where this is needed. 
See, for example, the test for "source_address" parameter of socket.create_connection() function.
msg104807 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-05-02 21:42
> See, for example, the test for "source_address" parameter of
> socket.create_connection() function.

Instead of testing the port, you could test the host. For example
by using two different loopback addresses (127.0.0.1 and 127.0.0.2?). Would it work on every platform?
msg104822 - (view) Author: Trent Nelson (trent) * (Python committer) Date: 2010-05-03 08:26
I introduced test_support.find_unused_port in r62234, as part of a general refactoring of client/server network-oriented tests.  The only reason I introduced this method at the time was because the SSL tests used to launch openssl in server mode, which required a port to be specified as part of the command line.

Pure Python tests shouldn't use this method, as noted in the docstring, they should use bind_port(), which takes platform socket-oddities into account.

I'm not sure if I agree that find_unused_port() should be removed though; it does serve a purpose in very rare corner cases.  Perhaps it should be renamed to _find_unused_port() to discourage accidental usage.

(I'd recommend reviewing r62234's commit log and change set for more info.)
msg104843 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2010-05-03 15:08
> I'm not sure if I agree that find_unused_port() should be removed though; it does serve a purpose in very rare corner cases.

It can serve a purpose in any number of cases.  My point is that it's *always* unreliable, though.  Any time you have any API that you want to test that requires a pre-allocated port number, you're going to have intermittent failures.  Such APIs are broken and should be fixed where possible and avoided otherwise.
msg104845 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2010-05-03 15:13
I've applied the logging patch and checked into trunk (r80712).
msg104891 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2010-05-03 22:04
> Any time you have any API that you want to test that requires a 
> pre-allocated port number, you're going to have intermittent failures.  
> Such APIs are broken and should be fixed where possible and avoided 
> otherwise.

You mean that socket.create_connection(), httplib (issue 3972) and ftplib (issue 8594) should have used a different API to implement their "source_address" option?
msg104893 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-05-03 22:08
Roundup ate my code. I meant:

>>> s = socket.create_connection(("python.org", 80), source_address=("192.168.0.2", 0))
>>> s.getsockname()
('192.168.0.2', 40496)
msg104951 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2010-05-04 15:49
> You mean that socket.create_connection(), httplib (issue 3972) and ftplib (issue 8594) should have used a different API to implement their "source_address" option?

I'm not sure what you mean.  The problem here is that you cannot reliably bind a specific local address because it might be bound already.

This is bad in unit tests because it causes spurious failures that waste developer effort forcing people to investigate non-bugs or because it causes people to ignore the results of the test suite.

This is bad in real applications because it prevents an application from working as it is supposed to.

Perhaps you have an application which requires that HTTP requests are issued from (1.2.3.4, 12345).  Then you'd better use an HTTP client library that lets you bind to this address when connecting to the HTTP server.  But the application is going to be prone to failure.  If you can't get around the requirement, then you just get to live with an unreliable application.

There's no such requirement in any unit test, though.  Unit tests can do whatever they want in order to achieve the goal of exercising the relevant implementation code and verify the results to be correct.  I don't think there are any unit tests which need to bind to one specific address in order to accomplish this goal.
msg104952 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2010-05-04 16:03
One of the tests in test_socket is checking that an attempt to connect to a port with no server running gives socket.error. For that, we need a port that's guaranteed to have no server present.

I think that one of the tests in test_httplib checks the source_address attribute to make sure the port it contains is the one you connected on - again, you need a specific port and it can't be in use. (Sorry, I'm not precise on the details of this one as it's only in trunk and I'm not at a PC with a trunk checkout at the moment).

Arguably these tests are of limited value and could simply be deleted, but they are there, and I don't see a way of implementing them without using something that gives you an unused port number...
msg104954 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2010-05-04 16:18
> One of the tests in test_socket is checking that an attempt to connect to a port with no server running gives socket.error. For that, we need a port that's guaranteed to have no server present.

A good way to do this is to create a socket, bind it to an address (any address - ie ('', 0)), ask it what its address is, and use that as the destination of the connect attempt.  Since you didn't listen() on it, it's not a server, and it won't accept any connections.

Doing it this way is again more reliable, since if you try to discover an unused address with find_unused_port, a real server might end up re-using that address by the time your test gets around to trying to connect to it.

> I think that one of the tests in test_httplib checks the source_address attribute to make sure the port it contains is the one you connected on - again, you need a specific port and it can't be in use.

If you're talking about SourceAddressTest.testHTTPConnectionSourceAddress, I see what you mean.  This is basically an integration test between httplib and the socket module.  No actual socket is required here.  I would implement this test using a fake socket object.  This would allow the test assertion to be strengthened (it currently doesn't assert anything about the ip part of the address, it only checks the port), and still avoids running into problems binding a *real* local address.

> Arguably these tests are of limited value and could simply be deleted

I definitely would not argue this.  These tests seem to cover real functionality which isn't directly covered elsewhere.  They shouldn't be deleted, but fixed to avoid the potential races they're subject to now.
msg104955 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2010-05-04 16:30
Thanks for the suggestions, I'll see if I can implement something based on them.
msg219947 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-06-07 15:41
msg104677, msg104822 and msg104845 refer to various commits but msg104955 suggests that a follow up is needed so where do we stand with this issue?
msg219950 - (view) Author: Paul Moore (paul.moore) * (Python committer) Date: 2014-06-07 16:27
TBH, I don't think I ever took this any further. As noted, the earlier patches fixed the failures I was hitting.

It looks like Python 3.4 now has *two* definitions of find_unused_port, in test.test_support and in test.support. And test_asyncio and test_ftplib also use the function now.
msg236969 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2015-03-01 17:28
find_unused_port is only defined in test.support. It is tested in test.test_support.

This solution might not be ideal but if it ain't broke, don't fix it?
msg380463 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2020-11-06 18:15
There are still several tests that use find_unused_port:

test_asyncio/test_events.py
test_asyncio/test_proactor_events.py
test_asyncio/test_sendfile.py
test_asyncio/test_unix_events.py
test_ftplib.py
test_httplib.py
test_largefile.py
test_socket.py
test_smtplib.py
History
Date User Action Args
2022-04-11 14:57:00adminsetgithub: 52822
2020-11-06 18:15:25iritkatrielsetnosy: + iritkatriel

messages: + msg380463
versions: + Python 3.8, Python 3.9, Python 3.10, - Python 2.6, Python 3.1, Python 2.7, Python 3.2
2019-03-15 22:39:27BreamoreBoysetnosy: - BreamoreBoy
2015-03-01 17:28:57BreamoreBoysetmessages: + msg236969
2014-06-07 22:27:49vinay.sajipsetnosy: - vinay.sajip
2014-06-07 16:28:24paul.mooresetnosy: - paul.moore
2014-06-07 16:27:52paul.mooresetmessages: + msg219950
2014-06-07 16:23:37brian.curtinsetnosy: - brian.curtin
2014-06-07 15:41:52BreamoreBoysetnosy: + BreamoreBoy
messages: + msg219947
2010-05-04 16:30:48paul.mooresetmessages: + msg104955
2010-05-04 16:18:18exarkunsetmessages: + msg104954
2010-05-04 16:03:11paul.mooresetmessages: + msg104952
2010-05-04 15:49:11exarkunsetmessages: + msg104951
2010-05-03 22:08:55pitrousetmessages: - msg104892
2010-05-03 22:08:48pitrousetmessages: + msg104893
2010-05-03 22:08:14pitrousetmessages: + msg104892
2010-05-03 22:04:08giampaolo.rodolasetmessages: + msg104891
2010-05-03 22:03:00giampaolo.rodolasetmessages: - msg104890
2010-05-03 22:02:39giampaolo.rodolasetmessages: + msg104890
2010-05-03 15:13:42vinay.sajipsetmessages: + msg104845
2010-05-03 15:08:42exarkunsetmessages: + msg104843
2010-05-03 08:26:40trentsetnosy: + trent
messages: + msg104822
2010-05-02 21:42:13pitrousetmessages: + msg104807
2010-05-02 21:38:20giampaolo.rodolasetnosy: + giampaolo.rodola
messages: + msg104806
2010-05-01 15:15:47paul.mooresetmessages: + msg104724
2010-05-01 13:53:51r.david.murraysetnosy: + r.david.murray
messages: + msg104715
2010-05-01 11:08:48pitrousetnosy: + vinay.sajip
messages: + msg104710
2010-05-01 10:26:59paul.mooresetfiles: + test_logging.patch

messages: + msg104701
2010-04-30 23:26:22pitrousetmessages: + msg104677
2010-04-30 16:30:30paul.mooresetfiles: + unused1.patch
keywords: + patch
messages: + msg104651
2010-04-30 14:18:32exarkunsetmessages: + msg104638
2010-04-30 10:00:42pitrousetnosy: + exarkun, tim.golden, brian.curtin

type: behavior
versions: + Python 2.6, Python 3.1, Python 2.7, Python 3.2
2010-04-30 09:19:03paul.mooresetnosy: + pitrou
2010-04-30 09:18:02paul.moorecreate