-
Notifications
You must be signed in to change notification settings - Fork 189
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 historical behavioural oddity with UNIX domain sockets #460
Conversation
…sed UNIX domain sockets inspired by https://stackoverflow.com/a/13719866
Network/Socket/SockAddr.hs
Outdated
res <- try (G.bind s a) | ||
case res of | ||
Right () -> pure () | ||
Left e -> if not (isAlreadyInUseError (e :: IOException)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't you use | isAlreadyInUseError (e :: IOException)
instead of if
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 8d5ff8d - though I've kept the error conditions grouped together at the top as I think that's visually easier to read, hope that's OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't forget pure
in tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woops, also done now in f27b87c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Rebased and merged. Thank you for your contribution! |
Originally part of #428. Hopefully the code & commit messages should be self-explanatory. TD;LR version is that bind doesn't clean up after itself, this works around that.