Nonblocking connect using poll #211
Conversation
Co-authored-by: Mark Rivers <rivers@cars.uchicago.edu>
ericonr
force-pushed
the
nonblocking-connect-poll
branch
from
f1a6f5e to
f44ddc8
Compare
| pasynManager->getAutoConnectTimeout(&connectTimeout); | ||
| msConnectTimeout = 1000 * connectTimeout; |
There was a problem hiding this comment.
One thing to note, which I don't know if it's relevant, is that any timeouts below 1ms will be rounded down to 0, causing poll to return immediately.
I don't think anyone actually would have such strict timeout requirements, though.
|
❌ Build asyn 1.0.283 failed (commit ae16b99eb5 by @ericonr) |
|
The failure seems unrelated: https://ci.appveyor.com/project/MarkRivers/asyn/builds/51101760/job/iyv53p19fkkop0vd |
|
❌ Build asyn 1.0.284 failed (commit b8bbe39652 by @ericonr) |
|
@ericonr I would like to merge this soon. Can you create a Github issue that describes the problem this PR is designed to fix, and some simple observations that will confirm that it is working? I think it fixes a problem like this: If ip_address is not reachable then the asynOcterSetInputEos() asynOcterSetOutputEos() commands will each hang for a long time because the port is locked waiting for the connect() to time out. Is this correct? |
This implementation is intended to allow faster forward progress in a startup script in cases where an unreachable device would keep an asyn port locked until the OS timed out the connection attempt (meaning the OS didn't have any other way of diagnosing the fact that the device was unreachable). Forward progress for the drvAsynIPPortConfigure command was already guaranteed, because the connection attempt happens in the port thread, and the main thread would only wait for autoConnectTimeout to see if the connection was successful. Since the asyn port would remain locked, subsequent commands to the port (e.g. asynSetOption, asynOctetSetInputEos, asynOctetSetOutputEos) would block until said OS timeout. Now, we apply the same autoConnectTimeout to the connection attempt itself. This was tested by comparing to the behavior from before the patch when connecting to "2.3.4.5:4001", where commands to the port would hang indefinitely. Use poll as a reasonably portable option. This takes advantage of the existing WSAPoll and FAKE_POLL fallback implementations as well. getsockopt(SO_ERROR) on Windows uses "int", removing the requirement for any custom code for that platform. We don't add any handling for EINTR, since it would add considerably more code, and the connection can simply be retried. For that reason, poll returning -1 is always an error.
ericonr
force-pushed
the
nonblocking-connect-poll
branch
from
f44ddc8 to
e198706
Compare
|
Hi @MarkRivers I have updated the commit message to include a better description of the problem being solved, as well as what was used to confirm it's working. Should I still open an issue, or is the commit description enough?
That is correct! |
|
❌ Build asyn 1.0.296 failed (commit 7ab23bcecf by @ericonr) |
|
@MarkRivers @ericonr There is already an open issue for this (#103). I have backported the patches to our local branch and rolled it out to some of our IOCs. Works fine and no problems so far. Thanks for your work to fix this! |
4 participants
Replacing #210