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.

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.

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.

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

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

Vinay, are you ok with the proposed patch?

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

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

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

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

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

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

I've applied the logging patch and checked into trunk (r80712).

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

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)

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

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

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

Thanks for the suggestions, I'll see if I can implement something based on them.

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?

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.

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?

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