Improvement in TCP/IP server

Better handling of the stored client sockets, to avoid accidental closure of the master socket

Change-Id: I5061e2702d9b537c1a0822a37ce9106b4d5d7110
Reviewed-by: Arttu Tarkiainen <arttu.tarkiainen@qt.io>

3 files changed, 36 insertions, 60 deletions

diff --git a/src/libs/qlicenseservice/licenser.cpp b/src/libs/qlicenseservice/licenser.cpp
index 3c72277..574fedd 100644
--- a/src/libs/qlicenseservice/licenser.cpp
+++ b/src/libs/qlicenseservice/licenser.cpp
@@ -52,7 +52,7 @@ Licenser::~Licenser()
 int Licenser::listen()
 {
     m_infoString = "";
-    int socket = 0; // Placeholder for whatever socket gets active
+    uint16_t socket = 0; // Placeholder for whatever socket gets active
     std::string input = m_tcpServer->listenToClients(socket);
     input = utils::trimStr(input);
 
@@ -62,7 +62,7 @@ int Licenser::listen()
         }
         return 0; //continue;
     }
-    if (input == CLIENT_CLOSED_MSG) {
+    if (input == SOCKET_CLOSED_MSG) {
         std::cout << "Client disconnected, socket id " << socket << std::endl;
         removeReservation(socket);
         return 0;
@@ -311,14 +311,14 @@ void Licenser::removeReservation(int socketId) {
                 m_currentClient = client;
                 if (sendServerRequest(reply) != e_bad_connection) {
                     // if connection succeeds, we can forget about this client
-                    std::cout << "Removing floating reservation with socket ID " << client->getSocketId() << std::endl;
+                    std::cout << "Removing floating reservation with ID " << client->getReservationId() << std::endl;
                     m_floatingClients.erase(m_floatingClients.begin() + i);
                     delete client;
                 }
             }
             return;
         } else if (client->removeChildClient(socketId)) {
-            std::cout << "Removed child client from reservation ID " << client->getReservationId() << std::endl;
+            std::cout << "Removed child client under reservation ID " << client->getReservationId() << std::endl;
             m_tcpServer->closeClientSocket(socketId);
             return;
         }
diff --git a/src/libs/qlicenseservice/tcpserver.cpp b/src/libs/qlicenseservice/tcpserver.cpp
index 646b6d4..330ad07 100644
--- a/src/libs/qlicenseservice/tcpserver.cpp
+++ b/src/libs/qlicenseservice/tcpserver.cpp
@@ -10,10 +10,6 @@ namespace QLicenseService {
 TcpServer::TcpServer(uint16_t serverPort)
 {
     int opt = TRUE;
-    // Initialize all m_clientSocket[] to 0 so not checked
-    for (uint8_t i = 0; i < MAX_CLIENTS; i++) {
-        m_clientSocket[i] = 0;
-    }
 
 #ifdef _WIN32
     // Initialize Winsock
@@ -56,7 +52,6 @@ TcpServer::TcpServer(uint16_t serverPort)
         exit(EXIT_FAILURE);
     }
     std::cout << "Listening to port " << serverPort << std::endl;
-   unsigned long mode = 1;
    //ioctlsocket(m_masterSocket, FIONBIO, &mode);
     // Try to specify maximum of 3 pending connections for the master socket
     if (listen(m_masterSocket, 3) < 0) {
@@ -73,14 +68,14 @@ TcpServer::TcpServer(uint16_t serverPort)
 TcpServer::~TcpServer()
 {
     doCloseSocket(m_masterSocket);
-    for (uint8_t i = 0; i < MAX_CLIENTS; i++) {
-        doCloseSocket(m_clientSocket[i]);
+    for (type_socket sd: m_clientSocket) {
+        doCloseSocket(sd);
     }
-
+    m_clientSocket.clear();
 }
 
 
-std::string TcpServer::listenToClients(int &socket)
+std::string TcpServer::listenToClients(uint16_t &socketId)
 {
     std::string data;
 
@@ -93,10 +88,7 @@ std::string TcpServer::listenToClients(int &socket)
     FD_SET(m_masterSocket, &m_readfds);
 
     // Add child sockets to set
-    for (int i = 0; i < MAX_CLIENTS; i++) {
-        // Socket descriptor
-        type_socket sd = m_clientSocket[i];
-
+    for (type_socket sd : m_clientSocket) {
         // If valid socket descriptor then add to read list
         if (sd > 0)
             FD_SET(sd, &m_readfds);
@@ -113,7 +105,7 @@ std::string TcpServer::listenToClients(int &socket)
     if ((activity < 0) && (errno != EINTR)) {
         std::cout << "select error \n";
     }
-    // If something happened on the master socket, then its an incoming connection
+    // If anything happens on the master socket, its an incoming connection
     if (FD_ISSET(m_masterSocket, &m_readfds)) {
         type_socket new_socket;
         if ((new_socket = accept(m_masterSocket,
@@ -121,21 +113,13 @@ std::string TcpServer::listenToClients(int &socket)
             perror("accept");
             exit(EXIT_FAILURE);
         }
-        // Add new socket to array of sockets
-        for (int i = 0; i < MAX_CLIENTS; i++) {
-            // If position is free
-            if (m_clientSocket[i] == 0) {
-                m_clientSocket[i] = new_socket;
-                socket = i;
-                std::cout << "New connection - added to list of sockets with id " << i << std::endl;
-                break;
-            }
-        }
+        // Add new socket to set of sockets
+        m_clientSocket.insert(new_socket);
+        std::cout << "New connection - added to list of sockets with id " << new_socket << std::endl;
     }
-    // Else its some IO operation on some other socket
-    for (int i = 0; i < MAX_CLIENTS; i++) {
-        type_socket sd = m_clientSocket[i];
 
+    // Else its some IO operation on some other socket
+    for (type_socket sd : m_clientSocket) {
         if (FD_ISSET(sd, &m_readfds)) {
             // Check if it was for closing , and also read the
             // incoming message
@@ -148,56 +132,45 @@ std::string TcpServer::listenToClients(int &socket)
                 // Somebody disconnected, get his details
                 getpeername(sd, (struct sockaddr *)&m_address,
                             (socklen_t *)&m_addrlen);
-                // Close the socket and mark as 0 in list for reuse
-                socket = i;
-                //doCloseSocket(sd, true); // Close and free from the list
-                data = "closed";
+                socketId = sd;
+                data = SOCKET_CLOSED_MSG;
             } else {
                 // Set the string terminating NULL byte
                 // on the end of the data read
                 m_buffer[valread] = '\0';
-                socket = i;
+                socketId = sd;
                 data = m_buffer;
             }
         }
     }
-
     return data;
 }
 
-int TcpServer::sendResponse(int socketIndex, const std::string &message)
+int TcpServer::sendResponse(int socketId, const std::string &message)
 {
-    type_socket sd = m_clientSocket[socketIndex];
-    if (send(sd, message.c_str(), (int)message.length(), 0) != message.length()) {
+    if (send((type_socket)socketId, message.c_str(), (int)message.length(), 0) != message.length()) {
         printf("[TcpServer] Send failed\n");
         return 1;
     }
     return 0;
 }
 
-void TcpServer::closeClientSocket(int socket)
+void TcpServer::closeClientSocket(int socketId)
 {
-    type_socket socketFD = m_clientSocket[socket];
-    std::cout << "Closing socket " << (int)socketFD << " (id: " << socket << ")\n";
-    doCloseSocket(socketFD, true);
+    std::cout << "Closing socket " << socketId << std::endl;
+    doCloseSocket(socketId);
+    m_clientSocket.erase(socketId);
 }
 
-void TcpServer::doCloseSocket(type_socket &socketFD, bool free)
+void TcpServer::doCloseSocket(type_socket &socketFD)
 {
 #if __APPLE__ || __MACH__  || __linux__
     close(socketFD);
 #else
     closesocket(socketFD);
 #endif
-    // Make sure to free the socket:
-    if (free) {
-        for (uint8_t index = 0; index < MAX_CLIENTS; index++ ) {
-            if (m_clientSocket[index] == socketFD) {
-                m_clientSocket[index] = 0;
-                break;
-            }
-        }
-    }
+
 }
 
+
 } // namespace QLicenseService
diff --git a/src/libs/qlicenseservice/tcpserver.h b/src/libs/qlicenseservice/tcpserver.h
index 16ced4a..22c3014 100644
--- a/src/libs/qlicenseservice/tcpserver.h
+++ b/src/libs/qlicenseservice/tcpserver.h
@@ -10,12 +10,15 @@
 #include <errno.h>
 #include <sys/types.h>
 #include <string.h>
+#include <unordered_set>
 
-#define MAX_CLIENTS 100
+
+#define BUFFER_SIZE 1024
 #define TRUE 1
 #define FALSE 0
+#define SOCKET_CLOSED_MSG  "closed"
+#define SOCKET_MAX_REACHED "clients_maxed_out"
 
-#define CLIENT_CLOSED_MSG  "closed"
 
 #if __APPLE__ || __MACH__ || __linux__
     #include <sys/types.h>
@@ -36,7 +39,6 @@
     typedef SOCKET type_socket;
 #endif
 
-#define BUFFER_SIZE 1024
 
 namespace QLicenseService {
 
@@ -46,19 +48,20 @@ public:
     explicit TcpServer(uint16_t serverPort);
     ~TcpServer();
 
-    std::string listenToClients(int &socket);
+    std::string listenToClients(uint16_t &socketId);
     int sendResponse(int socketIndex, const std::string &message);
     void closeClientSocket(int socketIndex);
 
 private:
     type_socket m_masterSocket;
-    type_socket m_clientSocket[MAX_CLIENTS];
+    std::unordered_set<type_socket> m_clientSocket;
     int m_addrlen;
     char m_buffer[BUFFER_SIZE]; // data buffer
     fd_set m_readfds; // set of socket descriptors
     struct sockaddr_in m_address;
 
-    void doCloseSocket(type_socket &socketFD, bool free=false);
+    void doCloseSocket(type_socket &socketFD);
+    bool generateSocketId(uint16_t &index);
 };
 
 } // namespace QLicenseService