Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

work around Windows Subsystem for Linux issue #5

Merged
merged 2 commits into from Mar 19, 2018
Merged

work around Windows Subsystem for Linux issue #5

merged 2 commits into from Mar 19, 2018

Conversation

Conduitry
Copy link
Contributor

@Conduitry Conduitry commented Mar 19, 2018

This isn't quiiiite an ideal solution. The idea is to attempt to open two servers on one port, and if that succeeds, assume that we're on WSL and switch future logic from using opening servers to using opening connections, which is slower.

The issue where WSL lets you open multiple servers on one port only seems to manifest if all of those servers were opened from within WSL. If one was already opened from within Windows itself, attempts to open one from within WSL fail with the expected EADDRINUSE. I'm guessing what's going on is that all of WSL is running as one Windows process and is taking care of the proxying to the virtual Linux processes itself - and so this is an understandable bug, but something I'd still consider a bug.

Anyway, what this means is that, with this PR, when we attempt to open two concurrent servers on one port, it might be that the first one was already opened in Windows, and that we still really are running in WSL - but in this case, we won't be able to determine that with this port. What I'm doing here is just assuming that means we're good to go (i.e., we're not on WSL) which isn't ideal. So either we need to check for WSL using a port that Definitely For Real Probably isn't in use by anything else that's likely to be running on the user's computer, or else we try with multiple different ports and keep going until we find one that we can open the first time (this is probably the best solution, it just would make this look uglier), or else we need to use a completely separate method for determining whether we're on WSL (which smells like an analogue of browser UA sniffing as opposed to feature detection, which isn't great).

Anyway, what there is here works better on WSL than what there was before (although not perfect). and it works marginally slower than before on non-WSL (because of the extra check at the beginning).

WSL lets you open multiple servers on the same port, so if we detect
that that seems to be the case, fall back to a slower method of
attempting to open connections to servers instead of attempting to
create servers.
@Conduitry
Copy link
Contributor Author

Actually, just realized that trying other ports for the WSL check isn't that bad if we're not worried about recursion - and we really shouldn't be, because having a bunch of consecutive occupied ports is really unlikely. I'm starting at 9000 (which was chosen fairly arbitrarily), and then we proceed on to 9001 etc. if we get an error on opening the first server. Pushed update. The tests pass on Windows and WSL, whether or not there is already something serving on port 9000.

@Conduitry
Copy link
Contributor Author

For future reference, it looks like this WSL issue was reported (quite recently!) here. From what's said in that thread, probably we shouldn't hold our breath for a solution.

@Rich-Harris Rich-Harris merged commit 05016b7 into Rich-Harris:master Mar 19, 2018
@Rich-Harris
Copy link
Owner

You absolute legend, thanks so much for figuring this out. I would have given up much sooner. Windows never disappoints

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants