amphp / dns

Async DNS resolution for PHP based on Amp.
https://amphp.org/dns
MIT License
157 stars 32 forks source link

Work around lack of connect timeout #33

Closed chrisleavoy closed 8 years ago

chrisleavoy commented 8 years ago

Fixes #32

mention-bot commented 8 years ago

By analyzing the blame information on this pull request, we identified @kelunik and @bwoebi to be potential reviewers

bwoebi commented 8 years ago

I rather meant to try STREAM_CLIENT_ASYNC_CONNECT. Your patch is doing a blocking connect. Currently working on it, I'll push shortly.

chrisleavoy commented 8 years ago

Thanks! I did experiment with STREAM_CLIENT_ASYNC_CONNECT after seeing it in amphp/socket, but it changed too much existing behavior for me to feel comfortable screwing with it. :)

bwoebi commented 8 years ago

Not much existing behavior is changed, see ba6016dd0a2a79e2eed8b2392ae3002d49120aaa

But thanks for finding our mistake… blocking connects are always bad!

bwoebi commented 8 years ago

Just as a note: when you do an async connect, you just have to wait for writability (this means: connected and buffers not full).