Problem With Sending Multiple Messages to Client

[joshblum]

Related to #11.

Following the ws_example.cpp the code below shows that sending multiple responses to a client is not handled properly. I want to be able to send multiple responses to the client so it can process them separately. Am I using the library incorrectly or is this a concurrency issue? The example seems to be independent of the variable NUM_THREADS, I think the problem is how the send function on the server handles the streams.

#include "server_ws.hpp"
#include "client_ws.hpp"

#define NUM_THREADS 4

using namespace std;

typedef SimpleWeb::SocketServer<SimpleWeb::WS> WsServer;
typedef SimpleWeb::SocketClient<SimpleWeb::WS> WsClient;

void send_message(WsServer* server, shared_ptr<WsServer::Connection> connection, std::string msg)
{
  cout << "Server: Sending Message: "  << msg << endl;
  auto send_stream=make_shared<WsServer::SendStream>();
  *send_stream << msg;
  //server.send is an asynchronous function
  server->send(connection, send_stream, [server, connection](const boost::system::error_code& ec){
      if(ec) {
      cout << "Server: Error sending message. " <<
      //See http://www.boost.org/doc/libs/1_55_0/doc/html/boost_asio/reference.html, Error Codes for error code meanings
      "Error: " << ec << ", error message: " << ec.message() << endl;
      }
      });
}

int main() {
  //WebSocket (WS)-server at port 8080 using 4 threads
  WsServer server(8080, NUM_THREADS);

  auto& test=server.endpoint["^/test/?$"];

  test.onmessage=[&server](shared_ptr<WsServer::Connection> connection, shared_ptr<WsServer::Message> message) {
    auto message_str=message->string();
    cout << "Server: Message received: \"" << message_str << "\" from " << (size_t)connection.get() << endl;
    send_message(&server, connection, "test1");
    send_message(&server, connection, "test2");
  };

  test.onopen=[](shared_ptr<WsServer::Connection> connection) {
    cout << "Server: Opened connection " << (size_t)connection.get() << endl;
  };

  //See RFC 6455 7.4.1. for status codes
  test.onclose=[](shared_ptr<WsServer::Connection> connection, int status, const string& reason) {
    cout << "Server: Closed connection " << (size_t)connection.get() << " with status code " << status << endl;
  };

  //See http://www.boost.org/doc/libs/1_55_0/doc/html/boost_asio/reference.html, Error Codes for error code meanings
  test.onerror=[](shared_ptr<WsServer::Connection> connection, const boost::system::error_code& ec) {
    cout << "Server: Error in connection " << (size_t)connection.get() << ". " << 
      "Error: " << ec << ", error message: " << ec.message() << endl;
  };

  thread server_thread([&server](){
      //Start WS-server
      server.start();
      });

  //Wait for server to start so that the client can connect
  this_thread::sleep_for(chrono::seconds(1));

  WsClient client("localhost:8080/test");
  client.onmessage=[&client](shared_ptr<WsClient::Message> message) {
    auto message_str=message->string();

    cout << "Client: Message received:" << message_str << endl;
  };

  client.onopen=[&client]() {
    cout << "Client: Opened connection" << endl;

    string message="Hello";
    cout << "Client: Sending message: \"" << message << "\"" << endl;

    auto send_stream=make_shared<WsClient::SendStream>();
    *send_stream << message;
    client.send(send_stream);
  };

  client.onclose=[](int status, const string& reason) {
    cout << "Client: Closed connection with status code " << status << endl;
  };

  //See http://www.boost.org/doc/libs/1_55_0/doc/html/boost_asio/reference.html, Error Codes for error code meanings
  client.onerror=[](const boost::system::error_code& ec) {
    cout << "Client: Error: " << ec << ", error message: " << ec.message() << endl;
  };

  client.start();

  server_thread.join();

  return 0;
}

[eidheim]

Thank you, it should be fixed now. I'll start work on custom streambufs soon, so that copying is avoided.

[eidheim]

Or rather, use http://www.boost.org/doc/libs/1_59_0/doc/html/boost_asio/reference/async_write/overload1.html somehow.

[eidheim]

I think the final solution solves the issue rather nicely.

Thank you again for letting me know of the problem, and giving a good test case where the server previously failed.

[joshblum]

Awesome! Happy to help, thank you so much for the fast responses :)

[joshblum]

Actually, I'm still getting the problem with the updated test case. The bug was most reproducible with NUM_THREADS = 1 and SIZE = 100000 and using a remote server (for longer network delay, I tried Chrome network throttling on my local machine but it didn't work as well) and testing in the browser. I tried in both Chrome and Firefox. The errors vary from

Client: A server must not mask any frames that it sends to the client. Server: Closed connection 139781388375136 with status code 1002

To Client: One or more reserved bits are on: reserved1 = 0, reserved2 = 0, reserved3 = 1 Server: Closed connection 140304971731392 with status code 1002

Any thoughts?

#include "server_ws.hpp"
#include "client_ws.hpp"

#define NUM_THREADS 1
#define SIZE 100000 // 400kB

using namespace std;

typedef SimpleWeb::SocketServer<SimpleWeb::WS> WsServer;
typedef SimpleWeb::SocketClient<SimpleWeb::WS> WsClient;

void send_message(WsServer* server, shared_ptr<WsServer::Connection> connection, std::string msg)
{
  cout << "Server: Sending Message: "  << msg << endl;
  auto send_stream = make_shared<WsServer::SendStream>();
  float* tmp = (float*) malloc(sizeof(float) * SIZE);
  send_stream->write((char*) tmp, SIZE);
  free(tmp);
  //server.send is an asynchronous function
  server->send(connection, send_stream, [server, connection](const boost::system::error_code & ec)
  {
    if (ec)
    {
      cout << "Server: Error sending message. " <<
           //See http://www.boost.org/doc/libs/1_55_0/doc/html/boost_asio/reference.html, Error Codes for error code meanings
           "Error: " << ec << ", error message: " << ec.message() << endl;
    }
  }, 130);
}

int main()
{
  //WebSocket (WS)-server at port 8080 using 4 threads
  WsServer server(8080, NUM_THREADS);

  auto& echo = server.endpoint["^/echo/?$"];

  echo.onmessage = [&server](shared_ptr<WsServer::Connection> connection, shared_ptr<WsServer::Message> message)
  {
    auto message_str = message->string();
    cout << "Server: Message received: \"" << message_str << "\" from " << (size_t)connection.get() << endl;
    for (int i = 0; i < 100; i ++)
    {
      send_message(&server, connection, "test-" + std::to_string(i));
    }
  };

  echo.onopen = [](shared_ptr<WsServer::Connection> connection)
  {
    cout << "Server: Opened connection " << (size_t)connection.get() << endl;
  };

  //See RFC 6455 7.4.1. for status codes
  echo.onclose = [](shared_ptr<WsServer::Connection> connection, int status, const string & reason)
  {
    cout << "Server: Closed connection " << (size_t)connection.get() << " with status code " << status << endl;
  };

  //See http://www.boost.org/doc/libs/1_55_0/doc/html/boost_asio/reference.html, Error Codes for error code meanings
  echo.onerror = [](shared_ptr<WsServer::Connection> connection, const boost::system::error_code & ec)
  {
    cout << "Server: Error in connection " << (size_t)connection.get() << ". " <<
         "Error: " << ec << ", error message: " << ec.message() << endl;
  };

  thread server_thread([&server]()
  {
    cout << "Started server" << endl;
    //Start WS-server
    server.start();
  });

//   //Wait for server to start so that the client can connect
//  this_thread::sleep_for(chrono::seconds(1));
//
//  WsClient client("localhost:8080/echo");
//  client.onmessage=[&client](shared_ptr<WsClient::Message> message) {
//    cout << "Client: Message received:" << endl;
//  };
//
//  client.onopen=[&client]() {
//    cout << "Client: Opened connection" << endl;
//
//    string message="Hello";
//    cout << "Client: Sending message: \"" << message << "\"" << endl;
//
//    auto send_stream=make_shared<WsClient::SendStream>();
//    *send_stream << message;
//    client.send(send_stream);
//  };
//
//  client.onclose=[](int status, const string& reason) {
//    cout << "Client: Closed connection with status code " << status << endl;
//  };
//
//  //See http://www.boost.org/doc/libs/1_55_0/doc/html/boost_asio/reference.html, Error Codes for error code meanings
//  client.onerror=[](const boost::system::error_code& ec) {
//    cout << "Client: Error: " << ec << ", error message: " << ec.message() << endl;
//  };
//
//  client.start();

  server_thread.join();

  return 0;
}

[eidheim]

I'll have a look at it Tomorrow morning, not sure what the problem is here, though I'll probably check if the message size sent to the client is correct (have done this before, but I might have missed a case).

[eidheim]

I'll also double check that it's not a timeout issue.

[eidheim]

Ah, there actually is a problem when sending several sends to the same client through for instance one onmessage since async_write is split up into several async_write_some, which might interleave. I have to fix it with strands (http://www.boost.org/doc/libs/1_59_0/doc/html/boost_asio/reference/io_service__strand.html). I'll try to fix it Tomorrow morning, if not I'll fix it on Saturday morning (I'm busy till then). The solution will be similar to the strand solution in https://github.com/eidheim/Simple-Web-Server, but not quite the same.

[eidheim]

By the way, thank you again for another excellent issue report:)

[joshblum]

No problem, thanks for helping debug these issues with me :)

[eidheim]

Fixing this on Saturday btw, got no time before sadly.

[eidheim]

I think the issue with sending several messages to the same client through for instance SocketServer::endpoint::onmessage should be fixed now.

[eidheim]

The client is now using message queue like the server as well. And some minor issues is fix on the server side as well. CMakeLists is also now including the correct boost components.

[joshblum]

Thank you so much!! Seems to be working for me now :)