Robustify TCPServer implementation for thread safety and windows support

Author: urfeex

include/ur_client_library/comm/socket_t.h

@@ -16,6 +16,7 @@
 
 #pragma once
 
+#include <system_error>
 #ifdef _WIN32
 
 #  define NOMINMAX
@@ -33,6 +34,7 @@
 
 typedef SOCKET socket_t;
 typedef SSIZE_T ssize_t;
+typedef int socklen_t;
 
 static inline int ur_setsockopt(socket_t s, int level, int optname, const void* optval, unsigned int optlen)
 {
@@ -64,3 +66,25 @@ typedef int socket_t;
 #  define ur_close close
 
 #endif  //  _WIN32
+
+#ifndef MSG_NOSIGNAL
+#  define MSG_NOSIGNAL 0
+#endif
+
+inline std::system_error makeSocketError(const std::string& message)
+{
+#ifdef _WIN32
+  return std::system_error(std::error_code(WSAGetLastError(), std::system_category()), message);
+#else
+  return std::system_error(std::error_code(errno, std::generic_category()), message);
+#endif
+}
+
+inline int getLastSocketError()
+{
+#ifdef _WIN32
+  return WSAGetLastError();
+#else
+  return errno;
+#endif
+}

include/ur_client_library/comm/tcp_server.h

@@ -32,6 +32,7 @@
 #include <atomic>
 #include <chrono>
 #include <functional>
+#include <mutex>
 #include <thread>
 #include <vector>
 
@@ -77,9 +78,14 @@ class TCPServer
    *
    * \param func Function handling the event information. The file descriptor created by the
    * connection event will be passed to the function.
+   *
+   * \note: For thread safety, this will block when there is an active callback around connection
+   * (connection, disconnection, shutdown). Thus, trying to call setConnectCallback e.g. by a
+   * handler function registered as disconnectCallback will result in a deadlock.
    */
   void setConnectCallback(std::function<void(const socket_t)> func)
   {
+    std::lock_guard<std::mutex> lk(clients_mutex_);
     new_connection_callback_ = func;
   }
 
@@ -88,9 +94,14 @@ class TCPServer
    *
    * \param func Function handling the event information. The file descriptor created by the
    * connection event will be passed to the function.
+   *
+   * \note: For thread safety, this will block when there is an active callback around connection
+   * (connection, disconnection, shutdown). Thus, trying to call setDisconnectCallback e.g. by a
+   * handler function registered as connectCallback will result in a deadlock.
    */
   void setDisconnectCallback(std::function<void(const socket_t)> func)
   {
+    std::lock_guard<std::mutex> lk(clients_mutex_);
     disconnect_callback_ = func;
   }
 
@@ -99,9 +110,13 @@ class TCPServer
    *
    * \param func Function handling the event information. The file client's file_descriptor will be
    * passed to the function as well as the actual message received from the client.
+   *
+   * \note: For thread safety, this will block when there is an active message callback. Thus, trying to call
+   * setMessageCallback e.g. from a handler function registered as messageCallback will result in a deadlock.
    */
   void setMessageCallback(std::function<void(const socket_t, char*, int)> func)
   {
+    std::lock_guard<std::mutex> lk(message_mutex_);
     message_callback_ = func;
   }
 
@@ -180,7 +195,7 @@ class TCPServer
   void handleDisconnect(const socket_t fd);
 
   //! read data from socket
-  void readData(const socket_t fd);
+  bool readData(const socket_t fd);
 
   //! Event handler. Blocks until activity on any client or connection attempt
   void spin();
@@ -200,6 +215,8 @@ class TCPServer
 
   uint32_t max_clients_allowed_;
   std::vector<socket_t> client_fds_;
+  std::mutex clients_mutex_;
+  std::mutex message_mutex_;
 
   static const int INPUT_BUFFER_SIZE = 4096;
   char input_buffer_[INPUT_BUFFER_SIZE];

include/ur_client_library/log.h

@@ -19,6 +19,7 @@
 #pragma once
 #include <inttypes.h>
 #include <memory>
+#include <system_error>
 
 #define URCL_LOG_DEBUG(...) urcl::log(__FILE__, __LINE__, urcl::LogLevel::DEBUG, __VA_ARGS__)
 #define URCL_LOG_WARN(...) urcl::log(__FILE__, __LINE__, urcl::LogLevel::WARN, __VA_ARGS__)
@@ -89,4 +90,18 @@ void setLogLevel(LogLevel level);
  */
 void log(const char* file, int line, LogLevel level, const char* fmt, ...);
 
+/*!
+ * \brief Cross-platform replacement for strerror.
+ *
+ * On MSVC, strerror triggers C4996 (deprecated). This function uses std::error_code instead,
+ * which is portable and thread-safe.
+ *
+ * \param errnum Error number (on POSIX typically errno)
+ * \returns Human-readable error message
+ */
+inline std::string strerrorPortable(int errnum)
+{
+  return std::error_code(errnum, std::system_category()).message();
+}
+
 }  // namespace urcl