Add synchronization to the network socket API

Add mutexes to protect the network socket API.  Also use semaphores to
wait for read/write events.  Also fix a typo in the comments for
timeout.

Author: c1728p9

Socket.cpp

@@ -19,109 +19,153 @@
 Socket::Socket()
     : _iface(0)
     , _socket(0)
-    , _timeout(-1)
+    , _timeout(osWaitForever)
 {
 }
 
 Socket::~Socket()
 {
-    if (_socket) {
-        close();
-    }
+    // Underlying close is thread safe
+    close();
 }
 
 int Socket::open(NetworkStack *iface, nsapi_protocol_t proto)
 {
+    _lock.lock();
+
+    if (_iface != NULL) {
+        _lock.unlock();
+        return NSAPI_ERROR_PARAMETER;
+    }
     _iface = iface;
 
     void *socket;
     int err = _iface->socket_open(&socket, proto);
     if (err) {
+        _lock.unlock();
         return err;
     }
 
     _socket = socket;
     _iface->socket_attach(_socket, &Socket::thunk, this);
 
+    _lock.unlock();
+
     return 0;
 }
 
 int Socket::close()
 {
-    if (!_socket) {
-        return 0;
+    _lock.lock();
+
+    int ret = 0;
+    if (_socket) {
+        _iface->socket_attach(_socket, 0, 0);
+
+        void * socket = _socket;
+        _socket = 0;
+        ret = _iface->socket_close(socket);
     }
-    
-    _iface->socket_attach(_socket, 0, 0);
-    
-    void *volatile socket = _socket;
-    _socket = 0;
-    return _iface->socket_close(socket);
+
+    // Wakeup anything in a blocking operation
+    // on this socket
+    socket_event();
+
+    _lock.unlock();
+    return ret;
 }
 
 int Socket::bind(uint16_t port)
 {
+    // Underlying bind is thread safe
     SocketAddress addr(0, port);
     return bind(addr);
 }
 
 int Socket::bind(const char *address, uint16_t port)
 {
+    // Underlying bind is thread safe
     SocketAddress addr(address, port);
     return bind(addr);
 }
 
 int Socket::bind(const SocketAddress &address)
 {
-    if (!_socket) {
-        return NSAPI_ERROR_NO_SOCKET;
+    _lock.lock();
+
+    int ret = NSAPI_ERROR_NO_SOCKET;
+    if (_socket) {
+        ret = _iface->socket_bind(_socket, address);
     }
 
-    return _iface->socket_bind(_socket, address);
+    _lock.unlock();
+    return ret;
 }
 
 void Socket::set_blocking(bool blocking)
 {
+    // Socket::set_timeout is thread safe
     set_timeout(blocking ? -1 : 0);
 }
 
 void Socket::set_timeout(int timeout)
 {
-    _timeout = timeout;
+    _lock.lock();
+
+    if (timeout >= 0) {
+        _timeout = (uint32_t)timeout;
+    } else {
+        _timeout = osWaitForever;
+    }
+
+    _lock.unlock();
 }
 
 int Socket::setsockopt(int level, int optname, const void *optval, unsigned optlen)
 {
-    if (!_socket) {
-        return NSAPI_ERROR_NO_SOCKET;
+    _lock.lock();
+
+    int ret = NSAPI_ERROR_NO_SOCKET;
+    if (_socket) {
+        ret = _iface->setsockopt(_socket, level, optname, optval, optlen);
     }
 
-    return _iface->setsockopt(_socket, level, optname, optval, optlen);
+    _lock.unlock();
+    return ret;
 }
 
 int Socket::getsockopt(int level, int optname, void *optval, unsigned *optlen)
 {
-    if (!_socket) {
-        return NSAPI_ERROR_NO_SOCKET;
+    _lock.lock();
+
+    int ret = NSAPI_ERROR_NO_SOCKET;
+    if (_socket) {
+        ret = _iface->getsockopt(_socket, level, optname, optval, optlen);
     }
 
-    return _iface->getsockopt(_socket, level, optname, optval, optlen);
+    _lock.unlock();
+    return ret;
 
 }
 
-void Socket::wakeup()
+void Socket::attach(FunctionPointer callback)
 {
+    _lock.lock();
+
+    _callback = callback;
+
+    _lock.unlock();
 }
 
 void Socket::thunk(void *data)
 {
     Socket *self = (Socket *)data;
-    if (self->_callback) {
-        self->_callback();
-    }
+    self->socket_event();
 }
 
-void Socket::attach(FunctionPointer callback)
+void Socket::socket_event(void)
 {
-    _callback = callback;
+    if (_callback) {
+        _callback();
+    }
 }

Socket.h

@@ -19,6 +19,7 @@
 
 #include "SocketAddress.h"
 #include "NetworkStack.h"
+#include "Mutex.h"
 
 /** Abstract socket class
  */
@@ -97,10 +98,11 @@ class Socket {
      *
      *  Initially all sockets have unbounded timeouts. NSAPI_ERROR_WOULD_BLOCK
      *  is returned if a blocking operation takes longer than the specified
-     *  timeout. A timeout of -1 removes the timeout from the socket.
+     *  timeout. A timeout of 0 removes the timeout from the socket. A negative
+     *  value give the socket an unbounded timeout.
      *
-     *  set_timeout(-1) is equivalent to set_blocking(false)
-     *  set_timeout(0) is equivalent to set_blocking(true)
+     *  set_timeout(0) is equivalent to set_blocking(false)
+     *  set_timeout(-1) is equivalent to set_blocking(true)
      *
      *  @param timeout  Timeout in milliseconds
      */
@@ -169,12 +171,13 @@ class Socket {
     int open(NetworkStack *iface, nsapi_protocol_t proto);
 
     static void thunk(void *);
-    static void wakeup();
+    virtual void socket_event(void);
 
     NetworkStack *_iface;
     void *_socket;
-    int _timeout;
+    uint32_t _timeout;
     FunctionPointer _callback;
+    rtos::Mutex _lock;
 };
 
 #endif

TCPServer.cpp

@@ -17,11 +17,11 @@
 #include "TCPServer.h"
 #include "Timer.h"
 
-TCPServer::TCPServer()
+TCPServer::TCPServer(): _accept_sem(0)
 {
 }
 
-TCPServer::TCPServer(NetworkStack *iface)
+TCPServer::TCPServer(NetworkStack *iface): _accept_sem(0)
 {
     open(iface);
 }
@@ -33,43 +33,69 @@ int TCPServer::open(NetworkStack *iface)
 
 int TCPServer::listen(int backlog)
 {
-    if (!_socket) {
-        return NSAPI_ERROR_NO_SOCKET;   
+    _lock.lock();
+
+    int ret = NSAPI_ERROR_NO_SOCKET;
+    if (_socket) {
+        ret = _iface->socket_listen(_socket, backlog);
     }
 
-    return _iface->socket_listen(_socket, backlog);
+    _lock.unlock();
+    return ret;
 }
 
 int TCPServer::accept(TCPSocket *connection)
 {
-    mbed::Timer timer;
-    timer.start();
-    mbed::Timeout timeout;
-    if (_timeout >= 0) {
-        timeout.attach_us(&Socket::wakeup, _timeout * 1000);
-    }
-
-    if (connection->_socket) {
-        connection->close();
-    }
+    _lock.lock();
 
+    int ret = NSAPI_ERROR_NO_SOCKET;
     while (true) {
         if (!_socket) {
-            return NSAPI_ERROR_NO_SOCKET;   
+            ret = NSAPI_ERROR_NO_SOCKET;
+            break;
         }
 
         void *socket;
-        int err = _iface->socket_accept(&socket, _socket);
-        if (!err) {
+        ret = _iface->socket_accept(&socket, _socket);
+        if (0 == ret) {
+            connection->_lock.lock();
+
+            if (connection->_socket) {
+                connection->close();
+            }
+
             connection->_iface = _iface;
             connection->_socket = socket;
+            _iface->socket_attach(socket, &Socket::thunk, connection);
+
+            connection->_lock.unlock();
+            break;
         }
 
-        if (err != NSAPI_ERROR_WOULD_BLOCK
-            || (_timeout >= 0 && timer.read_ms() >= _timeout)) {
-            return err;
+        if (NSAPI_ERROR_WOULD_BLOCK == ret) {
+            int32_t count;
+
+            _lock.unlock();
+            count = _accept_sem.wait(_timeout);
+            _lock.lock();
+
+            if (count < 1) {
+                ret = NSAPI_ERROR_WOULD_BLOCK;
+                break;
+            }
         }
+    }
 
-        __WFI();
+    _lock.unlock();
+    return ret;
+}
+
+void TCPServer::socket_event()
+{
+    int32_t status = _accept_sem.wait(0);
+    if (status <= 1) {
+        _accept_sem.release();
     }
+
+    Socket::socket_event();
 }

TCPServer.h

@@ -20,6 +20,7 @@
 #include "Socket.h"
 #include "TCPSocket.h"
 #include "NetworkStack.h"
+#include "Semaphore.h"
 
 /** TCP socket server
   */
@@ -74,6 +75,9 @@ class TCPServer : public Socket {
      *  @return         0 on success, negative error code on failure
      */
     int accept(TCPSocket *connection);
+protected:
+    virtual void socket_event(void);
+    rtos::Semaphore _accept_sem;
 };
 
 #endif