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

crow::Crow<>::stop() stops asio::io_context too soon #498

Open
DisableAsync opened this issue Jul 15, 2022 · 5 comments
Open

crow::Crow<>::stop() stops asio::io_context too soon #498

DisableAsync opened this issue Jul 15, 2022 · 5 comments
Labels
bug Something isn't working

Comments

@DisableAsync
Copy link

If one calls crow::websocket::connection::close() followed by crow::Crow<>::stop() right away, they are probably doomed.

I only skimmed through the code. Looks like the issue was, crow::websocket::connection::close() uses asio's dispatch function so if called from a different thread other than within the one(s) running io_context loop, it only post the task to the queue. It will need some time for the io_context to pick it up and execute. And crow::Crow<>::stop() stops io_contexts immediately, so the tasks are untouched and the connections remain in a connected state.

@DisableAsync
Copy link
Author

A quick fix could be instead of invoking asio::io_context::stop() directly, we also dispatch the invocation.

Not sure if this has its own pitfalls, just couldn't think of something better ATM.

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

I think another solution would be to have the server wait for all websocket connections to be closed before calling stop()

@DisableAsync
Copy link
Author

I think another solution would be to have the server wait for all websocket connections to be closed before calling stop()

I have been thinking, do we have a convenient way to wait for certain events like the closing of WebSocket connection, in Crow or ASIO?

@The-EDev
Copy link
Member

The-EDev commented Aug 9, 2022

Not exactly, what we do have is a list of all open websocket connections. We can use that to check whether or not they were all closed before stopping the server.

@z16166
Copy link
Contributor

z16166 commented Dec 8, 2022

It can be attacked by malicious clients if they hold the connections for ever, to prevent the server from stopping.

I just give up another websocket library called "uWebSockets" and turn to CrowCpp, because uWebSockets can't safely stop, it always crashes on Windows, but the developer of uWebSockets says it's my fault.

Not exactly, what we do have is a list of all open websocket connections. We can use that to check whether or not they were all closed before stopping the server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants