industrial_core icon indicating copy to clipboard operation
industrial_core copied to clipboard

tcp server makeConnect() blocks forever

Open jrgnicho opened this issue 7 years ago • 4 comments

In the absence of a connection, the TcpServer::makeConnect() method blocks the main thread and it won't exit even when the program is terminated with Ctrl-C. I can get around this by placing calling makeConnect() from a second thread however this alternative feels unsafe as the an unhandled exception is thrown when the program exits. I ran my node in ubuntu 16.04 with ros-kinetic.

jrgnicho avatar Jul 07 '17 01:07 jrgnicho

One way to fix this is to put this line in a while with a select(..) and only accept(..) whenever select(..) returns instead of timing out (this has a slight chance of introducing a race-condition, so should be carefully tested).

gavanderhoorn avatar Jul 07 '17 07:07 gavanderhoorn

@gavanderhoorn I tried using the select(...) approach you had suggested but it would error out even when a client had already connected, I followed the manual here for guidance on the proper use of select

What worked for me was to add the following lines around the accept() call

    // disable blocking
    int opts = fcntl(this->getSrvrHandle(),F_GETFL,0);
    opts |= O_NONBLOCK;
    fcntl(this->getSrvrHandle(), F_SETFL, opts);

    // accepting connection
    rc = ACCEPT(this->getSrvrHandle(), NULL, NULL);

    // re-enable blocking
    opts &= ~O_NONBLOCK;
    fcnlt(this->getSrvrHandle(), F_SETFL, opts);

If this solution is desirable I could submit a PR however some of the unit tests did fail after making this change.

jrgnicho avatar Jul 07 '17 21:07 jrgnicho

@jrgnicho, this goes beyond my knowledge of the socket library. How did you come up with this solution? Is there a reference somewhere?

Can you provide a test for this? I'm not entirely sure how you issue a Ctrl+C programmatically, but it might be possible.

shaun-edwards avatar Jul 08 '17 22:07 shaun-edwards

Here is a link that describes how to disable blocking. However, I decided not to use that approach since it caused some of the unit tests in simple message to fail. My final solution was to call makeConnect() from a second thread and then call shutdown on the socket handle from the main thread. Perhaps we can add a close method to TcpServer that just calls shutdown on the server handle.

jrgnicho avatar Jul 09 '17 15:07 jrgnicho