Fix "Address already in use" errors from test_introducer on POSIX #501
Conversation
create_tub on POSIX can pre-allocate a port safely instead.
This avoids leaking it into any child processes that the tests might launch.
Codecov Report
@@ Coverage Diff @@
## master #501 +/- ##
==========================================
+ Coverage 90.19% 90.23% +0.03%
==========================================
Files 144 144
Lines 27466 27466
Branches 3939 3939
==========================================
+ Hits 24773 24783 +10
+ Misses 1954 1947 -7
+ Partials 739 736 -3
Continue to review full report at Codecov.
|
exarkun
force-pushed
the
1595.address-already-in-use-test_introducer-create_tub
branch
from
e2b9dd7
to
318eea0
Compare
| if portnum is None: | ||
| from twisted.internet import reactor | ||
| from twisted.internet.interfaces import IReactorSocket | ||
| if IReactorSocket.providedBy(reactor): |
There was a problem hiding this comment.
We could probably at least move the interface import to the top? (I'm guessing reactor is imported here "in case something was trying to set a custom reactor"..?)
| # actual socket to an address. Once the bind succeeds here, we're | ||
| # no longer subject to any future EADDRINUSE problems. | ||
| import fcntl | ||
| s = socket() |
There was a problem hiding this comment.
import to top? Or is the IReactorSocket part telling us "we're on POSIX so the import can only succeed there"..?
| else: | ||
| # Get a random port number and fall through. This is necessary on | ||
| # Windows where Twisted doesn't offer IReactorSocket. This | ||
| # approach is error prone for the reasons described on |
There was a problem hiding this comment.
Ah, my previous comment's question is answered :) -- so maybe a comment that the IReactorSocket check is also telling us if we're POSIX vs Windows?
There was a problem hiding this comment.
Yea ... for now, anyway. I suppose if someone eventually implements IReactorSocket for some reactor on Windows then this might no longer be the case. And, indeed, I believe fcntl is not available on Windows (https://docs.python.org/2/library/index.html puts it in the "Unix Specific Services" section which I take as a hint in this direction as well).
There was a problem hiding this comment.
I am reminded of twisted.python.reflect.requireModule, though, which supports an idiom I prefer for maybe-not-available modules. So I'll do that.
exarkun
deleted the
1595.address-already-in-use-test_introducer-create_tub
branch
Fixes https://tahoe-lafs.org/trac/tahoe-lafs/ticket/1595 except on Windows (which lacks IReactorSocket support on Python 2). I'll open a new ticket for tracking the Windows fix.