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.

Sign up for GitHub

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

Jump to bottom

Websockets cause segfault when closing from server side #446

Closed
MichaelSB opened this issue May 27, 2022 · 15 comments · Fixed by #452
Closed

Websockets cause segfault when closing from server side #446

MichaelSB opened this issue May 27, 2022 · 15 comments · Fixed by #452
Labels
bug Something isn't working

Comments

@MichaelSB
Copy link
Contributor

MichaelSB commented May 27, 2022 •
edited by The-EDev
Loading

When I call crow::websocket::connection::close() server side, my application segfaults. Minimal example showing the problem I have:

#include "crow.h"
#include "crow/http_response.h"
#include "crow/json.h"
#include "crow/logging.h"
#include "crow/routing.h"
#include "crow/websocket.h"

int main(int, char**)
{
    crow::SimpleApp app;

    crow::websocket::connection* wsconn = nullptr;

    CROW_ROUTE(app, "/")([](const crow::request& req) {
        crow::response resp;
        resp.set_static_file_info("index.html");
        return resp;
    });

    CROW_ROUTE(app, "/ws").websocket()
        .onopen([&](crow::websocket::connection& conn) {
            wsconn = &conn;
        });
        .onclose([](crow::websocket::connection& conn, const std::string& reason) {
            CROW_LOG_INFO << reason;
        });

    CROW_ROUTE(app, "/disconnect")([&](const crow::request& req) {
        if(wsconn) {
            wsconn->close();
            wsconn = nullptr;
        }
        return crow::response(200);
    });

    app.port(18080).multithreaded().run();

    return 0;
}

index.html used:

<!DOCTYPE HTML>
<html>
<head>
  <meta charset="UTF-8">
  <title>Main</title>
</head>
<body>
<button type="button" onclick="disconnect()">Disconnect</button>
<script>
    var ws = new WebSocket("ws://localhost:18080/ws");

    function disconnect() {
        var req = new XMLHttpRequest();
        req.open("GET", "http://localhost:18080/disconnect", false);
        req.send(null);
    }
</script>
</body>
</html>

So the frontend opens a websocket and when the disconnect-button is clicked, the server should close it. I guess, I am doing something wrong, besides multiple connections cannot be handled in this example :) The onclose()-handler is called, I get reason logged. And then segfault.
@MichaelSB
Copy link
Contributor Author

v1.0+3 fwiw

@The-EDev The-EDev added the bug Something isn't working label May 27, 2022
@The-EDev
Copy link
Member

I can confirm the issue is occurring on Master as well, Will investigate further.

I should note you had an extra ; right before onclose which shouldn't be there.

@MichaelSB
Copy link
Contributor Author

Thanks. Sorry for the stray ;
I added onclose while reporting the issue and made a mistake with copy/paste.

@The-EDev
Copy link
Member

Don't worry about it :)

@The-EDev
Copy link
Member

Interestingly this issue seems to occur whenever the server attempts to close the websocket connection.. A temporary solution would be to close it from the client instead.

@MichaelSB
Copy link
Contributor Author

In a more elaborate program I got a segfault when closing the websocket server-side as well, but with a different backtrace. In that case, the method check_destroy of websocket::connection is called twice. This sounds similar to #355 (comment) I don't know, how or even if this is related to this issue.
Closing the socket client side works afaict. In my usecase it would be much nicer to let the server close the connection though.

@The-EDev The-EDev changed the title Websockets: how to close from server side (get segfault) Websockets cause segfault when closing from server side May 28, 2022
@The-EDev
Copy link
Member

When I ran the example with GDB, the stack trace showed that somehow it was attempting to read from the websocket after deleting it...

@MichaelSB
Copy link
Contributor Author

If I understand this correctly a read after delete could also cause check_destroy() being called a second time. At least I also see error-handler being called. So both segfaults I get could point to the same thing.

@The-EDev
Copy link
Member

I believe the actual problem is that read attempts to reference the socket_adaptor, which gets deleted along with the websocket connection. This is most likely the cause of the segfault.

@MichaelSB
Copy link
Contributor Author

I get the additional read after closing from the client-side too. I connected onopen, onerror and onclose, just logging the actions using CROW_LOG_.... In the log I find

(2022-06-01 10:09:39) [INFO    ] Websocket: client a571d98f-042d-4c67-a36c-97b7ea9c9746 open
(2022-06-01 10:09:40) [INFO    ] Websocket: client a571d98f-042d-4c67-a36c-97b7ea9c9746 close ()
(2022-06-01 10:11:28) [ERROR   ] Websocket: error

So onerror is triggered after onclose. I had a breakpoint in onerror to capture the stacktrace, that's why there's some time in between.
The stacktrace is pretty long and looks like:

#0  operator() (__closure=0x2c80032d138, conn=...) at /home/sb/Programming/example3/example.cc:85
#1  0x0000000000417551 in std::__invoke_impl<void, main(int, char**)::<lambda(crow::websocket::connection&)>&, crow::websocket::connection&>(std::__invoke_other, struct {...} &) (__f=...)
    at /usr/include/c++/10/bits/invoke.h:60
#2  0x0000000000416855 in std::__invoke_r<void, main(int, char**)::<lambda(crow::websocket::connection&)>&, crow::websocket::connection&>(struct {...} &) (__fn=...) at /usr/include/c++/10/bits/invoke.h:110
#3  0x000000000041569d in std::_Function_handler<void(crow::websocket::connection&), main(int, char**)::<lambda(crow::websocket::connection&)> >::_M_invoke(const std::_Any_data &, crow::websocket::connection &) (
    __functor=..., __args#0=...) at /usr/include/c++/10/bits/std_function.h:291
#4  0x000000000048015f in std::function<void (crow::websocket::connection&)>::operator()(crow::websocket::connection&) const (this=0x2c80032d138, __args#0=...) at /usr/include/c++/10/bits/std_function.h:622
#5  0x0000000000480303 in crow::websocket::Connection<crow::SocketAdaptor, crow::Crow<> >::do_read()::{lambda(boost::system::error_code const&, unsigned long)#1}::operator()(boost::system::error_code const&, unsigned long) const (this=0x2c80032c000, ec=...) at /home/sb/Programming/example3/external/crow/include/crow/websocket.h:335
#6  0x0000000000487176 in boost::asio::detail::binder2<crow::websocket::Connection<crow::SocketAdaptor, crow::Crow<> >::do_read()::{lambda(boost::system::error_code const&, unsigned long)#1}, boost::system::error_code, unsigned long>::operator()() (this=0x7fffee5042d0) at /usr/include/boost/asio/detail/bind_handler.hpp:164
#7  0x0000000000486303 in boost::asio::asio_handler_invoke<boost::asio::detail::binder2<crow::websocket::Connection<crow::SocketAdaptor, crow::Crow<> >::do_read()::{lambda(boost::system::error_code const&, unsigned long)#1}, boost::system::error_code, unsigned long> >(boost::asio::detail::binder2<crow::websocket::Connection<crow::SocketAdaptor, crow::Crow<> >::do_read()::{lambda(boost::system::error_code const&, unsigned long)#1}, boost::system::error_code, unsigned long>&, ...) (function=...) at /usr/include/boost/asio/handler_invoke_hook.hpp:69
#8  0x00000000004853b9 in boost_asio_handler_invoke_helpers::invoke<boost::asio::detail::binder2<crow::websocket::Connection<crow::SocketAdaptor, crow::Crow<> >::do_read()::{lambda(boost::system::error_code const&, unsigned long)#1}, boost::system::error_code, unsigned long>, crow::websocket::Connection<crow::SocketAdaptor, crow::Crow<> >::do_read()::{lambda(boost::system::error_code const&, unsigned long)#1}>(boost::asio::detail::binder2<crow::websocket::Connection<crow::SocketAdaptor, crow::Crow<> >::do_read()::{lambda(boost::system::error_code const&, unsigned long)#1}, boost::system::error_code, unsigned long>&, crow::websocket::Connection<crow::SocketAdaptor, crow::Crow<> >::do_read()::{lambda(boost::system::error_code const&, unsigned long)#1}&) (function=..., context=...) at /usr/include/boost/asio/detail/handler_invoke_helpers.hpp:37
#9  0x000000000048469d in boost::asio::detail::handler_work<crow::websocket::Connection<crow::SocketAdaptor, crow::Crow<> >::do_read()::{lambda(boost::system::error_code const&, unsigned long)#1}, boost::asio::system_executor>::complete<boost::asio::detail::binder2<crow::websocket::Connection<crow::SocketAdaptor, crow::Crow<> >::do_read()::{lambda(boost::system::error_code const&, unsigned long)#1}, boost::system::error_code, unsigned long> >(boost::asio::detail::binder2<crow::websocket::Connection<crow::SocketAdaptor, crow::Crow<> >::do_read()::{lambda(boost::system::error_code const&, unsigned long)#1}, boost::system::error_code, unsigned long>&, crow::websocket::Connection<crow::SocketAdaptor, crow::Crow<> >::do_read()::{lambda(boost::system::error_code const&, unsigned long)#1}&) (this=0x7fffee5042ff, function=..., handler=...) at /usr/include/boost/asio/detail/handler_work.hpp:82
#10 0x0000000000483a7d in boost::asio::detail::reactive_socket_recv_op<boost::asio::mutable_buffers_1, crow::websocket::Connection<crow::SocketAdaptor, crow::Crow<> >::do_read()::{lambda(boost::system::error_code const&, unsigned long)#1}>::do_complete(void*, boost::asio::detail::scheduler_operation*, boost::system::error_code const&, unsigned long) (owner=0x2c80027c200, base=0x2c800249110)
    at /usr/include/boost/asio/detail/reactive_socket_recv_op.hpp:122
#11 0x0000000000420f8c in boost::asio::detail::scheduler_operation::complete (this=0x2c800249110, owner=0x2c80027c200, ec=..., bytes_transferred=0) at /usr/include/boost/asio/detail/scheduler_operation.hpp:40
#12 0x0000000000424985 in boost::asio::detail::epoll_reactor::descriptor_state::do_complete (owner=0x2c80027c200, base=0x2c80026c900, ec=..., bytes_transferred=1) at /usr/include/boost/asio/detail/impl/epoll_reactor.ipp:776
#13 0x0000000000420f8c in boost::asio::detail::scheduler_operation::complete (this=0x2c80026c900, owner=0x2c80027c200, ec=..., bytes_transferred=1) at /usr/include/boost/asio/detail/scheduler_operation.hpp:40
#14 0x00000000004255fb in boost::asio::detail::scheduler::do_run_one (this=0x2c80027c200, lock=..., this_thread=..., ec=...) at /usr/include/boost/asio/detail/impl/scheduler.ipp:401
#15 0x0000000000424fa6 in boost::asio::detail::scheduler::run (this=0x2c80027c200, ec=...) at /usr/include/boost/asio/detail/impl/scheduler.ipp:154
#16 0x0000000000425b0f in boost::asio::io_context::run (this=0x2c800224820) at /usr/include/boost/asio/impl/io_context.ipp:62
#17 0x000000000044963d in crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}::operator()() const (this=0x2c8002d8000) at /home/sb/Programming/example3/external/crow/include/crow/http_server.h:113
#18 0x000000000047b9aa in std::__invoke_impl<void, crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}>(std::__invoke_other, crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}&&) (__f=...)
    at /usr/include/c++/10/bits/invoke.h:60
#19 0x000000000047b8a3 in std::__invoke<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}>(crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}&&) (__fn=...)
    at /usr/include/c++/10/bits/invoke.h:95
#20 0x000000000047b5c0 in std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >::_M_invoke<0ul>(std::_Index_tuple<0ul>) (this=0x2c8002e8048) at /usr/include/c++/10/thread:264
#21 0x000000000047af0a in std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >::operator()() (this=0x2c8002e8048) at /usr/include/c++/10/thread:271
#22 0x000000000047a40b in std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >, void>::operator()() const (this=0x7fffee504c30) at /usr/include/c++/10/future:1373
#23 0x000000000047962a in std::__invoke_impl<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >, void>&>(std::__invoke_other, std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >, void>&) (__f=...)
    at /usr/include/c++/10/bits/invoke.h:60
#24 0x0000000000478493 in std::__invoke_r<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter>, std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >, void>&>(std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >, void>&) (__fn=...) at /usr/include/c++/10/bits/invoke.h:115
#25 0x0000000000476f4a in std::_Function_handler<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> (), std::__future_base::_Task_setter<std::unique_ptr<std::__future_base::_Result<void>, std::__future_base::_Result_base::_Deleter>, std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >, void> >::_M_invoke(std::_Any_data const&) (__functor=...)
    at /usr/include/c++/10/bits/std_function.h:292
#26 0x00000000004399ed in std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>::operator()() const (this=0x7fffee504c30) at /usr/include/c++/10/bits/std_function.h:622
#27 0x0000000000429a25 in std::__future_base::_State_baseV2::_M_do_set(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*) (this=0x2c8002e8010, 
    __f=0x7fffee504c30, __did_set=0x7fffee504ba7) at /usr/include/c++/10/future:572
#28 0x000000000044d0f7 in std::__invoke_impl<void, void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::__invoke_memfun_deref, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__fu--Type <RET> for more, q to quit, c to continue without paging--c
ture_base::_Result_base::_Deleter> ()>*&&, bool*&&) (__f=@0x7fffee504bc0: (void (std::__future_base::_State_baseV2::*)(std::__future_base::_State_baseV2 * const, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter>()> *, bool *)) 0x4299fe <std::__future_base::_State_baseV2::_M_do_set(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*)>, __t=@0x7fffee504bb8: 0x2c8002e8010) at /usr/include/c++/10/bits/invoke.h:73
#29 0x0000000000442b45 in std::__invoke<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) (__fn=@0x7fffee504bc0: (void (std::__future_base::_State_baseV2::*)(std::__future_base::_State_baseV2 * const, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter>()> *, bool *)) 0x4299fe <std::__future_base::_State_baseV2::_M_do_set(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*)>) at /usr/include/c++/10/bits/invoke.h:95
#30 0x00000000004397b8 in std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#1}::operator()() const (this=0x7fffee504b40) at /usr/include/c++/10/mutex:717
#31 0x00000000004397e3 in std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::operator()() const (this=0x0) at /usr/include/c++/10/mutex:722
#32 0x00000000004397f4 in std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&)::{lambda()#2}::_FUN() () at /usr/include/c++/10/mutex:722
#33 0x00007ffff6ea15a7 in __pthread_once_slow () from /lib64/libpthread.so.0
#34 0x0000000000411f24 in __gthread_once (__once=0x2c8002e8028, __func=0x40f610 <__once_proxy@plt>) at /usr/include/c++/10/x86_64-suse-linux/bits/gthr-default.h:700
#35 0x0000000000439880 in std::call_once<void (std::__future_base::_State_baseV2::*)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*>(std::once_flag&, void (std::__future_base::_State_baseV2::*&&)(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*), std::__future_base::_State_baseV2*&&, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*&&, bool*&&) (__once=..., __f=@0x7fffee504bc0: (void (std::__future_base::_State_baseV2::*)(std::__future_base::_State_baseV2 * const, std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter>()> *, bool *)) 0x4299fe <std::__future_base::_State_baseV2::_M_do_set(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>*, bool*)>) at /usr/include/c++/10/mutex:729
#36 0x000000000042984f in std::__future_base::_State_baseV2::_M_set_result(std::function<std::unique_ptr<std::__future_base::_Result_base, std::__future_base::_Result_base::_Deleter> ()>, bool) (this=0x2c8002e8010, __res=..., __ignore_failure=false) at /usr/include/c++/10/future:412
#37 0x0000000000473e19 in std::__future_base::_Async_state_impl<std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >, void>::_Async_state_impl(std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >&&)::{lambda()#1}::operator()() const (this=0x2c8002e8010) at /usr/include/c++/10/future:1682
#38 0x0000000000480c24 in std::__invoke_impl<void, std::__future_base::_Async_state_impl<std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >, void>::_Async_state_impl(std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >&&)::{lambda()#1}>(std::__invoke_other, std::__future_base::_Async_state_impl<std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >, void>::_Async_state_impl(std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >&&)::{lambda()#1}&&) (__f=...) at /usr/include/c++/10/bits/invoke.h:60
#39 0x000000000047f9ef in std::__invoke<std::__future_base::_Async_state_impl<std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >, void>::_Async_state_impl(std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >&&)::{lambda()#1}>(std::__future_base::_Async_state_impl<std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >, void>::_Async_state_impl(std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >&&)::{lambda()#1}&&) (__fn=...) at /usr/include/c++/10/bits/invoke.h:95
#40 0x000000000047f3ce in std::thread::_Invoker<std::tuple<std::__future_base::_Async_state_impl<std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >, void>::_Async_state_impl(std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >&&)::{lambda()#1}> >::_M_invoke<0ul>(std::_Index_tuple<0ul>) (this=0x2c8002248c8) at /usr/include/c++/10/thread:264
#41 0x000000000047e064 in std::thread::_Invoker<std::tuple<std::__future_base::_Async_state_impl<std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >, void>::_Async_state_impl(std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >&&)::{lambda()#1}> >::operator()() (this=0x2c8002248c8) at /usr/include/c++/10/thread:271
#42 0x000000000047c10c in std::thread::_State_impl<std::thread::_Invoker<std::tuple<std::__future_base::_Async_state_impl<std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >, void>::_Async_state_impl(std::thread::_Invoker<std::tuple<crow::Server<crow::Crow<>, crow::SocketAdaptor>::run()::{lambda()#1}> >&&)::{lambda()#1}> > >::_M_run() (this=0x2c8002248c0) at /usr/include/c++/10/thread:215
#43 0x00007ffff6b563b4 in ?? () from /usr/lib64/libstdc++.so.6
#44 0x00007ffff6e986ea in start_thread () from /lib64/libpthread.so.0
#45 0x00007ffff6239a8f in clone () from /lib64/libc.so.6

This does not segfault, though obviously check_destroy is called in line 336 of websocket.h after the connection was already destroyed by the close. Line 335 in websocket.h is where the onerror-handler with the log-message is triggered.

@MichaelSB
Copy link
Contributor Author

MichaelSB commented Jun 1, 2022 •
edited
Loading

Why it even gets this far in this case, I have no idea. It should trigger the segfault caused by using the already deleted socket_adaptor as well?

@MichaelSB
Copy link
Contributor Author

onerror is called ~1s after onclose. I timed it using std::chrono, because I noticed quite some delay between the two, even in a release build. This 1s delay is pretty solid, only a few µs difference between different runs. So this looks like some kind of timer?

@MichaelSB
Copy link
Contributor Author

MichaelSB commented Jun 2, 2022 •
edited by The-EDev
Loading

I can avoid the segfault when closing server-side by changing the else in line 329 of websocket.h to else if(!close_connection_). This also avoids the error-handler being called after the close when closing client-side. This is no fix of course, there is still an attempted read after everything was destroyed. This seems to be triggered by http_server.h in line 113, but I am not sure, what to do with all this boost and async stuff :D

@MichaelSB
Copy link
Contributor Author

@The-EDev I think I found the issue. handle_fragment, called by do_read when closing the socket, calls check_destroy and the connection is deleted. But after handle_fragment returns, state_ is set to WebsocketReadState::MiniHeader and do_read is called again, then on the already deleted connection. This causes a segfault or valgrind getting upset on invalid reads/writes, depending on moonphase :) A possible fix could be to return bool from handle_fragment indicating whether a new read should be started or not. In case 0x8 (close) we would return false after check_destroy, true in all other cases. I tried this and it seems to work. What do you think?

@MichaelSB
Copy link
Contributor Author

This fixes the segfault when closing server-side. I still get an additional read with error-handler being called, when closing client-side. Investigating further, I found that the server is receiving a close, then according to https://datatracker.ietf.org/doc/html/rfc6455#section-5.5.1 sends back the close. It then does do_read, expecting an answer from the client. But the client already closed the connection, because: After both sending and receiving a Close message, an endpoint considers the WebSocket connection closed and MUST close the underlying TCP connection.. In the end do_read waiting for a miniheader fails with eof. It calls the error-handler and destroys the connection. I think, we should exit early in do_read, if both has_sent_close_ and has_recv_close_ are true, only destroying the connection, not even attempting a read in that case. I implemented this and it works for me. I'll issue a PR implementing both changes.

@MichaelSB MichaelSB mentioned this issue Jun 4, 2022
Merged
@The-EDev The-EDev linked a pull request Jun 4, 2022 that will close this issue
do not try to read from websocket after sending and receiving the clo… #452
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

do not try to read from websocket after sending and receiving the clo… MichaelSB/Crow
2 participants
@MichaelSB @The-EDev