microsoft / cpprestsdk

The C++ REST SDK is a Microsoft project for cloud-based client-server communication in native code using a modern asynchronous C++ API design. This project aims to help C++ developers connect to and interact with services.
Other
7.99k stars 1.65k forks source link

hostport_listener stop listening for connections after first call to on_accept with an error #742

Open sridmad opened 6 years ago

sridmad commented 6 years ago

The code in hostport_listener::on_accept does not subscribe to listening for new connections if it was called with an error. As a result a http based rest server stops accepting connections if an error occurs.

See https://www.boost.org/doc/libs/1_63_0/doc/html/boost_asio/tutorial.html#boost_asio.tutorial.tutdaytime3 for pattern.

garethsb commented 6 years ago

This seems true. I'm surprised it's not been reported before. Do you have an example or test that reproduces such an error happening?

sridmad commented 6 years ago

We have hit this issue on and off in our product and found the issue through code review and confirmed through logging. Unfortunately no consistent repro.

I have made the change in a fork. All Debug tests pass. Some tests fail in Release build but not consistently (i.e. different tests are failing on different runs). Not sure if it is due to the change or the release tests have the issues.

Will take a look at the test failures to see if they have a dependency on the existing pattern.

garethsb commented 6 years ago

That would be great.

Have you logged which error_code values are seen?

sridmad commented 6 years ago

Error code was not logged previously. We have since added code to log the error code to cpprestsdk and rebuilding our product. We should have the error code value(s) in a day or two.

garethsb commented 6 years ago

This looks strongly related to #546 which does have a repro and a suggested fix in #575. I haven't confirmed it myself.

sridmad commented 6 years ago

@garethsb-sony, #546 seems to handle one of the cases. I suspect there are other error cases that have to be handled.

https://github.com/chriskohlhoff/asio/blob/ad1751063aa717bd128dddc23456667ab816b2d9/asio/src/examples/cpp03/http/server/server.cpp seems to handle socket close case.