-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Occasional crash in client when server disconnects (std::logic_error: uninitialized stream object) #979
Comments
That it happens when the server disconnects is an interesting observation. As a short-term solution, I'd advice you to downgrade to cpprestsdk 2.10.2, which doesn't suffer from this problem. |
@reneme @BillyONeal When I use the startup cpprestsdk to connect to the HTTP server, loop the request server, then close the HTTP server, and then the same error occurs. Request:
Stacktrace:
Connect is closed: void handle_failed_read_status_line(const boost::system::error_code& ec, const char* generic_error_message)
{
if (m_connection->was_reused_and_closed_by_server(ec))
{
// Failed to write to socket because connection was already closed while it was in the pool.
// close() here ensures socket is closed in a robust way and prevents the connection from being put to the
// pool again.
m_connection->close();
// Create a new context and copy the request object, completion event and
// cancellation registration to maintain the old state.
// This also obtains a new connection from pool.
auto new_ctx = create_request_context(m_http_client, m_request);
new_ctx->m_request._get_impl()->instream().seek(0);
new_ctx->m_request_completion = m_request_completion;
new_ctx->m_cancellationRegistration = m_cancellationRegistration;
auto client = std::static_pointer_cast<asio_client>(m_http_client);
// Resend the request using the new context.
client->send_request(new_ctx);
}
else
{
report_error(generic_error_message, ec, httpclient_errorcode_context::readheader);
}
}
https://github.com/Microsoft/cpprestsdk/blob/v2.10.8/Release/include/cpprest/streams.h#L1083 void _verify_and_throw(const char *msg) const
{
auto buffer = helper()->m_buffer;
if ( !(buffer.exception() == nullptr) )
std::rethrow_exception(buffer.exception());
if ( !buffer.can_read() )
throw std::runtime_error(msg);
} Hotfix: diff --git a/Release/src/http/client/http_client_asio.cpp b/Release/src/http/client/http_client_asio.cpp
index 0ef90695..ef8c56a9 100644
--- a/Release/src/http/client/http_client_asio.cpp
+++ b/Release/src/http/client/http_client_asio.cpp
@@ -1322,7 +1322,10 @@ private:
// cancellation registration to maintain the old state.
// This also obtains a new connection from pool.
auto new_ctx = create_request_context(m_http_client, m_request);
- new_ctx->m_request._get_impl()->instream().seek(0);
+ if (new_ctx->m_request._get_impl()->instream().can_seek())
+ {
+ new_ctx->m_request._get_impl()->instream().seek(0);
+ }
new_ctx->m_request_completion = m_request_completion;
new_ctx->m_cancellationRegistration = m_cancellationRegistration; Test OK. |
I think we need to report that situation as an error because the request can't be meaningfully restarted without seek. |
For |
Sure, if there wasn't a body at all we shouldn't even be trying that. |
I opened a PR that builds on @fcharlie's hotfix and extends it with some error reporting at least. Some feedback would be appreciated. |
For those who can't wait for the solution to be released - setting the body to an empty string for GET requests worked for me (linux).
|
@zpalmtree Probably this could be closed with our PR merged into master. Please feel free to verify that it works for you now. |
@reneme Thank you for your work, unfortunately I ended up using another library as the crashes were too frequent for my use case. I'll reopen if I end up using the library again and hit this issue. |
FWIW, my team also found an easy repro for the exact exception shown by the OP in v2.10.8 and have confirmed as working correctly in v2.10.10. Thank you for the fix. |
I've got this library working well in my client, however, I'm having a rare exception thrown which I can't handle, when the server terminates. After closing the server, occasionally, a few seconds after, the client will terminate with:
This appears to be thrown from here: https://github.com/Microsoft/cpprestsdk/blob/2a19d84af30567ad6c1ce6d14ab13f89bb618d74/Release/include/cpprest/streams.h
Note that I am not running cpprestsdk on the server - It's a custom stack.
Now, I have no issue with it throwing if the server disconnects, that's to be expected. However, it's an exception thrown in another thread, so I am unable to catch it.
Most of the time, I'll get one of these exceptions:
All good - I can just put a catch around my .wait() or .get() and handle them.
However, with the std::logic_error, we can see from the backtrace that it is thrown in another thread, so there is no way for us to catch it (I think):
Am I doing something wrong, or is this a fault in the library? Here's a slightly simplified version of what I'm doing:
I'm running this code, and in another thread calling another function, which does basically the same thing. Is it possible this is due to me using multiple threads? It seems strange that disconnecting the server would cause the crash, whilst it does not crash at any other point.
The other function:
Possibly related to #857 - but I see that's been patched.
System info: Linux
Compiler: Clang-7
Library version: v2.10.8
Thanks.
The text was updated successfully, but these errors were encountered: