3
\$\begingroup\$

I'm writing a program and I have a server that needs to be able to handle multiple client connections at once doing many different things.

I'm using the Qt library, so I handle new connections like this:

In the server constructor:

Server::Server(QObject* parent): QObject(parent)
{
  connect(&server, SIGNAL(newConnection()), //&server is a QTcpServer object
          this, SLOT(acceptConnection())); 
  qDebug() << "[" << currentTime() << "] " << "Server started.";
  server.listen(QHostAddress::Any, PORT_NUMBER);
  qDebug() << "[" << currentTime() << "] " << "Server listening.";
}

The acceptConnection slot:

void Server::acceptConnection()
{
  client = server.nextPendingConnection(); //client is a QTcpSocket* object

  connect(client, SIGNAL(readyRead()), //When there is data to be read
    this, SLOT(startRead()));
}

The startRead slot:

void Server::startRead()
{

    char serverReceiveBuf[65536]; //A buffer for the data sent to the server
    client->read(serverReceiveBuf, client->bytesAvailable());
    handleConnection(serverReceiveBuf); //Do something with that data
}

I'm not going to give the handleConnection function because it's a bit long and I don't think it's necessary for the question.

My worry is latency. If I have one or two users, it's fine. But I haven't tested many concurrent users. There's a synchronization signal sent by the client that asks the server to verify the data the client has matches the data on the server. I'm sending this request every 250 ms, so with a few more concurrent users the client may end up waiting too long every 250 ms because of the other clients sending in synchronization requests.

At least, that's what I believe. Qt is pretty powerful, but I'm not entirely sure if when a signal is received that a new thread is automatically created to handle the connection. If I'm not mistaken, the current way I handle connections involves queueing, so that when the server receives a signal it'll process the requests in the order received. It's important to note that I don't call client->close() until the client terminates the connection themselves, so once client = server.nextPendingConnecting(); is called client will always exist until the client terminates it.

Is this a good way of handling multiple connections, given that I'll be regularly having users send synchronization requests often? If not, how should I handle the connections instead?

\$\endgroup\$
  • 1
    \$\begingroup\$ This question appears to be off-topic because it is about asking for advice on usage of qt. This would be better resolved on stackoverflow.com \$\endgroup\$ – Martin York Jul 22 '14 at 17:31
  • 3
    \$\begingroup\$ This question could be asked on Stack Overflow, but I don't think it's off-topic for Code Review. In such situations, I'd prefer to keep the question open. \$\endgroup\$ – 200_success Jul 22 '14 at 18:36
  • 1
    \$\begingroup\$ I guess I posted it here because I have working code that seems to do what I want it to but I wasn't sure if there was a better way to handle it. \$\endgroup\$ – Lighthat Jul 22 '14 at 20:15
  • 1
    \$\begingroup\$ An advantage with posting this on SO is that there are a lot more people on the main sight. CodeReview is a very small community. The C++ corner even quieter. My qt skill is old and rusty thus I am not really got much I can help with. \$\endgroup\$ – Martin York Jul 24 '14 at 17:10
  • \$\begingroup\$ @LokiAstari It looks like I'm going to have to. Thanks for letting me know this type of question is off-topic. \$\endgroup\$ – Lighthat Jul 25 '14 at 15:35
2
\$\begingroup\$

I was not able to find this question on SO, so let me try to give an answer here:

A general idea:

Depending on the nature of the data your are synchronizing, it might be better that the server sends out a synchronization signal and the clients synchronize to the data received from the server.

Specifics to the code of the question

I see two problems in the slot function acceptConnection() according to the documentation:

  1. the function nextPendingConnection() may return 0. This may lead to a problem with the following connect statement. You should check for 0 response.
  2. nextPendingConnection() will not return the exact same connection for which the signal was sent, it just returns "the next one". This may lead to a problem when multiple connections are made at the same time, or your code cannot handle one of them in time. It might be better that you handle all pending connections in a loop.

With these additions your code would perhaps look like this:

void Server::acceptConnection()
{
   while(client = server.nextPendingConnection()) //client is a QTcpSocket* 
   { 
      connect(client, SIGNAL(readyRead()), this, SLOT(startRead()));
   }
}

This brings us directly to the next problem: the client variable - I assume that this is a member variable of the Server class - will be overwritten for each new connection.

The Better approach is to follow the advice in the documentation and override incomingConnection(). We could subclass QTcpSocket and implement the handleConnection() functions from there. The documentation describes what the base class function normally does.

\$\endgroup\$
  • \$\begingroup\$ Hi, I've made lots of changes since I posted this. I've handled the issue with client being overwritten - I create a new client every time a connection is made and add it to a list of QTcpSocket* variables to prevent overwriting. I've also added a way to handle nextPendingConnection() returning 0. I have not done it in a loop though. I'll look more into overriding incomingConnection() instead. With regard to your general idea, I already send a signal from the server to the clients which contains synchronization data. \$\endgroup\$ – Lighthat Oct 14 '15 at 17:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.