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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix socket initialization #5059

Merged
merged 1 commit into from Oct 2, 2017

Conversation

bcardiff
Copy link
Member

When #4707 was merged, in particular c26fbc9 the socket initialization got screwed a little bit.

I notice it because the playground was not working in master in a manual test.

Socket initialization (TCPSocket at least) uses Addrinfo.tcp which may retry and call Socket#initialize, leaving inconsistent state since the socket is tracked as closed.

It is a code smell to use super in a closure probably 馃挱

Socket initialization (TCPSocket at least) uses Addrinfo.tcp which may retry and call Socket#initialize, leaving inconsistent state since the socket is tracked as closed
@bcardiff
Copy link
Member Author

The only way i see a spec can be added is either using localhost or some dns like lvh.me that resolves to 127.0.0.1

@bcardiff bcardiff requested a review from RX14 September 30, 2017 17:17
RX14
RX14 approved these changes Sep 30, 2017
@RX14
Copy link
Contributor

RX14 commented Sep 30, 2017

I agree that we should disallow super in loops and closures. This fix is fine for now though.

@bcardiff bcardiff merged commit b830899 into crystal-lang:master Oct 2, 2017
@RX14 RX14 added this to the Next milestone Oct 2, 2017
@bcardiff bcardiff deleted the fix/socket-initalization branch October 3, 2017 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants