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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tcp server makeConnect() blocks forever #177

Open
jrgnicho opened this issue Jul 7, 2017 · 4 comments
Open

tcp server makeConnect() blocks forever #177

jrgnicho opened this issue Jul 7, 2017 · 4 comments

Comments

@jrgnicho
Copy link
Member

jrgnicho commented Jul 7, 2017

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.

@gavanderhoorn
Copy link
Member

gavanderhoorn commented Jul 7, 2017

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).

@jrgnicho
Copy link
Member Author

jrgnicho commented Jul 7, 2017

@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.

@shaun-edwards
Copy link
Member

@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.

@jrgnicho
Copy link
Member Author

jrgnicho commented Jul 9, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants