Fix issues caused by invoking exit() on non-main thread

These could cause unpredictable behavior, resulting in crashes
if the daemon would be started twice, trying to bind the TCP
server socket to an address with already taken port, etc.

Fix by:

- Add generic runtime exception class for daemon thrown errors.

- Replace exit() calls that could occur on non-main thread with
  throwing the new exception.

- Add test case failing without the fix.

Change-Id: I5cfe679bdd9a034b1913edf30be00acfe3f3232c
Reviewed-by: Iikka Eklund <iikka.eklund@qt.io>

7 files changed, 121 insertions, 56 deletions

diff --git a/src/libs/qlicenseservice/errors.h b/src/libs/qlicenseservice/errors.h
new file mode 100644
index 0000000..fff21dd
--- /dev/null
+++ b/src/libs/qlicenseservice/errors.h
@@ -0,0 +1,33 @@
+/* Copyright (C) 2023 The Qt Company Ltd.
+ *
+ * SPDX-License-Identifier: GPL-3.0-only WITH Qt-GPL-exception-1.0
+*/
+
+#pragma once
+
+#include <stdexcept>
+#include <string>
+
+namespace QLicenseService {
+
+class Error : public std::runtime_error
+{
+public:
+    explicit Error(const char *message)
+        : std::runtime_error(message)
+    {}
+
+    explicit Error(const std::string &message)
+        : std::runtime_error(message)
+    {}
+
+    virtual ~Error() noexcept
+    {}
+
+    virtual const char *what() const noexcept override
+    {
+        return std::runtime_error::what();
+    }
+};
+
+} // namespace QLicenseService
diff --git a/src/libs/qlicenseservice/licenser.cpp b/src/libs/qlicenseservice/licenser.cpp
index 5d7c814..9094bd7 100644
--- a/src/libs/qlicenseservice/licenser.cpp
+++ b/src/libs/qlicenseservice/licenser.cpp
@@ -11,6 +11,7 @@
 #include "clitoolhandler.h"
 #include "version.h"
 #include "utils.h"
+#include "errors.h"
 
 namespace QLicenseService {
 
@@ -36,6 +37,10 @@ Licenser::Licenser(uint16_t tcpPort, const std::string &settingsPath)
             m_settings->get("version_query_access_point"));
     // Start the TCP/IP server
     m_tcpServer = new TcpServer(tcpPort);
+    if (!m_tcpServer->init()) {
+        throw Error("Error while initializing TCP server in port: "
+            + std::to_string(tcpPort));
+    }
 }
 
 Licenser::~Licenser()
@@ -50,7 +55,10 @@ int Licenser::listen()
 {
     m_infoString = "";
     uint16_t socketId = 0; // Placeholder for whatever socket gets active
-    std::string input = m_tcpServer->listenToClients(socketId);
+    std::string input;
+    if (!m_tcpServer->listenToClients(socketId, &input))
+        throw QLicenseService::Error("Error while listening to clients!");
+
     input = utils::trimStr(input);
 
     if (input.empty()) {
diff --git a/src/libs/qlicenseservice/licenseservice.cpp b/src/libs/qlicenseservice/licenseservice.cpp
index 6e1d158..a9c83c3 100644
--- a/src/libs/qlicenseservice/licenseservice.cpp
+++ b/src/libs/qlicenseservice/licenseservice.cpp
@@ -6,6 +6,7 @@
 #include "licenseservice.h"
 
 #include "licenser.h"
+#include "errors.h"
 
 #if __linux__ || __APPLE__ || __MACH__
 #include <csignal>
@@ -31,10 +32,9 @@ void LicenseService::run()
         sigfillset(&signal_set);
         pthread_sigmask(SIG_BLOCK, &signal_set, NULL);
 #endif
-        Licenser licenser(m_tcpPort, m_settingsPath);
-
-        while (1) {
-            try {
+        try {
+            Licenser licenser(m_tcpPort, m_settingsPath);
+            while (1) {
                 licenser.listen();
                 if (isCancelRequested()) {
                     updateAndNotifyStatus(Status::Canceled);
@@ -42,16 +42,21 @@ void LicenseService::run()
                 } else if (status() != Status::Running) {
                     updateAndNotifyStatus(Status::Running);
                 }
-            } catch (const std::exception &e) {
-                setError("Caught exception: " + std::string(e.what()));
-                updateAndNotifyStatus(Status::Error);
-                return;
-            } catch (...) {
-                setError("Caught unknown exception");
-                updateAndNotifyStatus(Status::Error);
-                return;
             }
+        } catch (const QLicenseService::Error &e) {
+            setError("Caught exception: " + std::string(e.what()));
+            updateAndNotifyStatus(Status::Error);
+            return;
+        } catch (const std::exception &e) {
+            setError("Caught exception: " + std::string(e.what()));
+            updateAndNotifyStatus(Status::Error);
+            return;
+        } catch (...) {
+            setError("Caught unknown exception");
+            updateAndNotifyStatus(Status::Error);
+            return;
         }
+
         // Never reached
         updateAndNotifyStatus(Status::Finished);
 }
diff --git a/src/libs/qlicenseservice/tcpserver.cpp b/src/libs/qlicenseservice/tcpserver.cpp
index 330ad07..565f43a 100644
--- a/src/libs/qlicenseservice/tcpserver.cpp
+++ b/src/libs/qlicenseservice/tcpserver.cpp
@@ -8,6 +8,20 @@
 namespace QLicenseService {
 
 TcpServer::TcpServer(uint16_t serverPort)
+    : m_serverPort(serverPort)
+{
+}
+
+TcpServer::~TcpServer()
+{
+    doCloseSocket(m_masterSocket);
+    for (type_socket sd: m_clientSocket) {
+        doCloseSocket(sd);
+    }
+    m_clientSocket.clear();
+}
+
+bool TcpServer::init()
 {
     int opt = TRUE;
 
@@ -18,19 +32,19 @@ TcpServer::TcpServer(uint16_t serverPort)
     iResult = WSAStartup(MAKEWORD(2, 2), &wsaData);
     if (iResult != NO_ERROR) {
         wprintf(L"Error at WSAStartup()\n");
-        exit(EXIT_FAILURE);
+        return false;
     }
 #endif
     // Create a master socket
     if ((m_masterSocket = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP)) == 0) {
         perror("socket failed");
-        exit(EXIT_FAILURE);
+        return false;
     }
     // Set master socket to allow multiple connections
     if (setsockopt(m_masterSocket, SOL_SOCKET, SO_REUSEADDR, (char *)&opt,
                    sizeof(opt)) < 0) {
         perror("setsockopt failed");
-        exit(EXIT_FAILURE);
+        return false;
     }
     timeval tv {1, 0}; //t.tv_sec = 2; t.tv_usec = 0;
     int ret = setsockopt(m_masterSocket, SOL_SOCKET, SO_RCVTIMEO,
@@ -43,42 +57,31 @@ TcpServer::TcpServer(uint16_t serverPort)
     // Type of socket created
     m_address.sin_family = AF_INET;
     m_address.sin_addr.s_addr = INADDR_ANY;
-    m_address.sin_port = htons(serverPort);
+    m_address.sin_port = htons(m_serverPort);
 
     // Bind the socket to localhost
     if (bind(m_masterSocket, (struct sockaddr *)&m_address, sizeof(m_address)) < 0) {
         doCloseSocket(m_masterSocket);
         perror("bind failed");
-        exit(EXIT_FAILURE);
+        return false;
     }
-    std::cout << "Listening to port " << serverPort << std::endl;
+    std::cout << "Listening to port " << m_serverPort << std::endl;
    //ioctlsocket(m_masterSocket, FIONBIO, &mode);
     // Try to specify maximum of 3 pending connections for the master socket
     if (listen(m_masterSocket, 3) < 0) {
         doCloseSocket(m_masterSocket);
         perror("listen");
-        exit(EXIT_FAILURE);
+        return false;
     }
     m_addrlen = sizeof(m_address);
 
     // Accept the incoming connection
     std::cout << "Waiting for connections ...\n";
+    return true;
 }
 
-TcpServer::~TcpServer()
+bool TcpServer::listenToClients(uint16_t &socketId, std::string *data)
 {
-    doCloseSocket(m_masterSocket);
-    for (type_socket sd: m_clientSocket) {
-        doCloseSocket(sd);
-    }
-    m_clientSocket.clear();
-}
-
-
-std::string TcpServer::listenToClients(uint16_t &socketId)
-{
-    std::string data;
-
     // Clear the socket set
     FD_ZERO(&m_readfds);
 
@@ -111,7 +114,7 @@ std::string TcpServer::listenToClients(uint16_t &socketId)
         if ((new_socket = accept(m_masterSocket,
                 (struct sockaddr *)&m_address, (socklen_t *)&m_addrlen)) < 0) {
             perror("accept");
-            exit(EXIT_FAILURE);
+            return false;
         }
         // Add new socket to set of sockets
         m_clientSocket.insert(new_socket);
@@ -133,17 +136,17 @@ std::string TcpServer::listenToClients(uint16_t &socketId)
                 getpeername(sd, (struct sockaddr *)&m_address,
                             (socklen_t *)&m_addrlen);
                 socketId = sd;
-                data = SOCKET_CLOSED_MSG;
+                *data = SOCKET_CLOSED_MSG;
             } else {
                 // Set the string terminating NULL byte
                 // on the end of the data read
                 m_buffer[valread] = '\0';
                 socketId = sd;
-                data = m_buffer;
+                *data = m_buffer;
             }
         }
     }
-    return data;
+    return true;
 }
 
 int TcpServer::sendResponse(int socketId, const std::string &message)
diff --git a/src/libs/qlicenseservice/tcpserver.h b/src/libs/qlicenseservice/tcpserver.h
index 22c3014..ce4ce23 100644
--- a/src/libs/qlicenseservice/tcpserver.h
+++ b/src/libs/qlicenseservice/tcpserver.h
@@ -48,11 +48,13 @@ public:
     explicit TcpServer(uint16_t serverPort);
     ~TcpServer();
 
-    std::string listenToClients(uint16_t &socketId);
+    bool init();
+    bool listenToClients(uint16_t &socketId, std::string *data);
     int sendResponse(int socketIndex, const std::string &message);
     void closeClientSocket(int socketIndex);
 
 private:
+    uint16_t m_serverPort;
     type_socket m_masterSocket;
     std::unordered_set<type_socket> m_clientSocket;
     int m_addrlen;
diff --git a/src/windows_daemon/windowsdaemon.cpp b/src/windows_daemon/windowsdaemon.cpp
index d073685..10ab6f8 100644
--- a/src/windows_daemon/windowsdaemon.cpp
+++ b/src/windows_daemon/windowsdaemon.cpp
@@ -42,16 +42,15 @@ int __cdecl _tmain(int argc, TCHAR *argv[])
     if (lstrcmpi(argv[1], TEXT("--nodaemon")) == 0) {
             // Fire it up
         printf("Starting qtlicd as CLI tool\n");
-        Licenser licenser(0);
-        while (true)
-        {
-            try {
+
+        try {
+            Licenser licenser(0);
+            while (true)
                 licenser.listen();
-            }
-            catch (...) {
-                printf("ERROR: Qt License Daemon listen() failed!!\n");
-                return -1;
-            }
+        }
+        catch (...) {
+            printf("ERROR: Qt License Daemon listen() failed!!\n");
+            return -1;
         }
     }
     // If command-line parameter is "--version", show the version and exit.
@@ -255,18 +254,17 @@ VOID SvcInit( DWORD dwArgc, LPTSTR *lpszArgv)
 
     // Fire it up
     printf("Starting daemon\n");
-    Licenser licenser(0);
 
-    // Perform work until service stops.
-    while (WaitForSingleObject(ghSvcStopEvent, 0) != WAIT_OBJECT_0)
-    {
-        try {
+    try {
+        Licenser licenser(0);
+        // Perform work until service stops.
+        while (WaitForSingleObject(ghSvcStopEvent, 0) != WAIT_OBJECT_0)
             licenser.listen();
-        }
-        catch (...) {
-            printf("ERROR: Qt License Daemon listen() failed!!\n");
-        }
     }
+    catch (...) {
+        printf("ERROR: Qt License Daemon listen() failed!!\n");
+    }
+
     ReportSvcStatus( SERVICE_STOP_PENDING, NO_ERROR, 0 );
     CloseHandle(ghSvcStopEvent);
     ReportSvcStatus( SERVICE_STOPPED, NO_ERROR, 0 );
diff --git a/tests/auto/licenseservice/tst_licenseservice.cpp b/tests/auto/licenseservice/tst_licenseservice.cpp
index 80ec27b..2cda6a2 100644
--- a/tests/auto/licenseservice/tst_licenseservice.cpp
+++ b/tests/auto/licenseservice/tst_licenseservice.cpp
@@ -20,3 +20,19 @@ TEST_CASE("Start/stop without clients", "[licenseservice]")
     REQUIRE((service.cancel() && service.waitForFinished()));
     REQUIRE(service.status() == LicenseService::Status::Canceled);
 }
+
+TEST_CASE("Start two daemon instances with same port", "[licenseservice]")
+{
+    LicenseService service(0, DAEMON_SETTINGS_FILE_DEFAULT);
+    // 1. Verify starting of license "service" succeeds
+    REQUIRE((service.start() && service.waitForStarted()));
+
+    LicenseService service2(0, DAEMON_SETTINGS_FILE_DEFAULT);
+    // 2. Verify starting another instance fails gracefully
+    REQUIRE((service2.start() && !service2.waitForStarted()));
+    REQUIRE(service2.status() == LicenseService::Status::Error);
+
+    // 3. Verify stop original service
+    REQUIRE((service.cancel() && service.waitForFinished()));
+    REQUIRE(service.status() == LicenseService::Status::Canceled);
+}