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

Add optional keep-alive mechanism to WS connections #506

Open
paupuerta opened this issue Jul 22, 2022 · 3 comments
Open

Add optional keep-alive mechanism to WS connections #506

paupuerta opened this issue Jul 22, 2022 · 3 comments
Labels
feature Code based project improvement

Comments

@paupuerta
Copy link

paupuerta commented Jul 22, 2022

Hi

When the client shutdown gently, like powers off, a normal reboot or even unplugging the network, works well, but in case of client disconnect suddenly like halt force, reboot force, power down... the server keeps waiting to read forever.

websocket.h

	void do_read()
	{ 
	...
		adaptor_.socket().async_read_some (..... // here wait forever.
	...

Here is an abstract of my test server code.

CROW_ROUTE(app, "/ws")
      .websocket()
      .onopen([&](crow::websocket::connection &conn) {
          std::lock_guard<std::mutex> _(mtx);
          try {
            CROW_LOG_INFO << "new connection: " << conn.get_remote_ip();
            auto us = users.emplace(&conn, Context(cnf));
          } catch (const exception& ex) {
            CROW_LOG_INFO << "exception new connection";
          }
      })
      .onclose([&](crow::websocket::connection &conn, const std::string &reason) {
          std::lock_guard<std::mutex> _(mtx);
          try {
            CROW_LOG_INFO << "connection closed reason: " << reason << " "  << conn.get_remote_ip();
            users.erase(&conn);
          } catch (const exception& ex) {
            CROW_LOG_ERROR << "exception connection closed";
          }
      })
      .onerror([&](crow::websocket::connection &conn) {
         std::lock_guard<std::mutex> _(mtx);
         try {
            CROW_LOG_INFO << "connection error: " << conn.get_remote_ip();
            conn.close("ErrorDetected");
            //users.erase(&conn);
         } catch (const exception& ex) {
            CROW_LOG_ERROR << "exception onerror";
         }
      })
      .onmessage([&](crow::websocket::connection &conn, const std::string &data, bool is_binary) {
	 CROW_LOG_INFO << "Text format not implemented " << data;
      })

In the current CrowCpp (1.0+3 cause I use conan.io) I don't see how to avoid this (for now I have had to change websocket.h by adding a timer)

Is this a bug, or am I missing something?

@The-EDev
Copy link
Member

TCP connections (and websockets by extension) don't have a mechanism to detect a power outage. Usually keepalive functionality is implemented by the application. I've seen multiple websocket server require a ping signal every x seconds, otherwise the server terminates the connection.

This might be a thing worth adding to Crow in the future

@The-EDev The-EDev changed the title Dangling Websockets connections Add optional keep-alive mechanism to WS connections Jul 22, 2022
@The-EDev The-EDev added the feature Code based project improvement label Jul 22, 2022
@paupuerta
Copy link
Author

Great!

As an idea, I implemented it by adding another callback handler to the WebSocket connection so the protocol would be like the developer wants, either a ping/pong mechanism, close connection directly, or whatever.

If you want, I can share my approximation.

Regards

@The-EDev
Copy link
Member

I thought of an approach that uses an app parameter and the task_timer, but please feel free to share your version :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Code based project improvement
Projects
None yet
Development

No branches or pull requests

2 participants