Allow valid socket handles of 0 in StreamingSocket and DatagramSocket, add some unit tests and some minor docs cleanup

modules/juce_core/network/juce_Socket.cpp

@@ -87,7 +87,7 @@ namespace SocketHelpers
 
     static bool resetSocketOptions (SocketHandle handle, bool isDatagram, bool allowBroadcast) noexcept
     {
-        return handle > 0
+        return handle >= 0
                 && setOption (handle, SO_RCVBUF, (int) 65536)
                 && setOption (handle, SO_SNDBUF, (int) 65536)
                 && (isDatagram ? ((! allowBroadcast) || setOption (handle, SO_BROADCAST, (int) 1))
@@ -103,7 +103,7 @@ namespace SocketHelpers
        #if JUCE_WINDOWS
         ignoreUnused (portNumber, isListener, readLock);
 
-        if (h != (unsigned) SOCKET_ERROR || connected)
+        if (h != invalidSocket || connected)
             closesocket (h);
 
         // make sure any read process finishes before we delete the socket
@@ -122,7 +122,7 @@ namespace SocketHelpers
             }
         }
 
-        if (h != -1)
+        if (h >= 0)
         {
             // unblock any pending read requests
             ::shutdown (h, SHUT_RDWR);
@@ -147,7 +147,7 @@ namespace SocketHelpers
 
     static bool bindSocket (SocketHandle handle, int port, const String& address) noexcept
     {
-        if (handle <= 0 || ! isValidPortNumber (port))
+        if (handle < 0 || ! isValidPortNumber (port))
             return false;
 
         struct sockaddr_in addr;
@@ -163,7 +163,7 @@ namespace SocketHelpers
 
     static int getBoundPort (SocketHandle handle) noexcept
     {
-        if (handle > 0)
+        if (handle >= 0)
         {
             struct sockaddr_in addr;
             socklen_t len = sizeof (addr);
@@ -183,7 +183,7 @@ namespace SocketHelpers
         if (getpeername (handle, (struct sockaddr*) &addr, &len) >= 0)
             return inet_ntoa (addr.sin_addr);
 
-        return String ("0.0.0.0");
+        return "0.0.0.0";
     }
 
     static int readSocket (SocketHandle handle,
@@ -281,11 +281,14 @@ namespace SocketHelpers
             return -1;
        #else
         {
-            int result;
+            int result = 0;
 
-            while ((result = select (h + 1, prset, pwset, nullptr, timeoutp)) < 0
-                    && errno == EINTR)
+            for (;;)
             {
+                result = select (h + 1, prset, pwset, nullptr, timeoutp);
+
+                if (result >= 0 || errno != EINTR)
+                    break;
             }
 
             if (result < 0)
@@ -501,7 +504,8 @@ bool StreamingSocket::connect (const String& remoteHostName, int remotePortNumbe
 
     if (isListener)
     {
-        jassertfalse;    // a listener socket can't connect to another one!
+        // a listener socket can't connect to another one!
+        jassertfalse;
         return false;
     }
 
@@ -515,7 +519,10 @@ bool StreamingSocket::connect (const String& remoteHostName, int remotePortNumbe
     connected = SocketHelpers::connectSocket (handle, readLock, remoteHostName,
                                               remotePortNumber, timeOutMillisecs);
 
-    if (! (connected && SocketHelpers::resetSocketOptions (handle, false, false)))
+    if (! connected)
+        return false;
+
+    if (! SocketHelpers::resetSocketOptions (handle, false, false))
     {
         close();
         return false;
@@ -526,7 +533,8 @@ bool StreamingSocket::connect (const String& remoteHostName, int remotePortNumbe
 
 void StreamingSocket::close()
 {
-    SocketHelpers::closeSocket (handle, readLock, isListener, portNumber, connected);
+    if (handle >= 0)
+        SocketHelpers::closeSocket (handle, readLock, isListener, portNumber, connected);
 
     hostName.clear();
     portNumber = 0;
@@ -631,8 +639,11 @@ void DatagramSocket::shutdown()
 
     std::atomic<int> handleCopy { handle.load() };
     handle = -1;
+
     std::atomic<bool> connected { false };
     SocketHelpers::closeSocket (handleCopy, readLock, false, 0, connected);
+
+    isBound = false;
 }
 
 bool DatagramSocket::bindToPort (int port)
@@ -644,6 +655,9 @@ bool DatagramSocket::bindToPort (int port, const String& addr)
 {
     jassert (SocketHelpers::isValidPortNumber (port));
 
+    if (handle < 0)
+        return false;
+
     if (SocketHelpers::bindSocket (handle, port, addr))
     {
         isBound = true;
@@ -722,7 +736,7 @@ int DatagramSocket::write (const String& remoteHostname, int remotePortNumber,
 
 bool DatagramSocket::joinMulticast (const String& multicastIPAddress)
 {
-    if (! isBound || handle < 0)
+    if (handle < 0 || ! isBound)
         return false;
 
     return SocketHelpers::multicast (handle, multicastIPAddress, lastBindAddress, true);
@@ -730,7 +744,7 @@ bool DatagramSocket::joinMulticast (const String& multicastIPAddress)
 
 bool DatagramSocket::leaveMulticast (const String& multicastIPAddress)
 {
-    if (! isBound || handle < 0)
+    if (handle < 0 || ! isBound)
         return false;
 
     return SocketHelpers::multicast (handle, multicastIPAddress, lastBindAddress, false);
@@ -738,7 +752,7 @@ bool DatagramSocket::leaveMulticast (const String& multicastIPAddress)
 
 bool DatagramSocket::setMulticastLoopbackEnabled (bool enable)
 {
-    if (! isBound || handle < 0)
+    if (handle < 0 || ! isBound)
         return false;
 
     return SocketHelpers::setOption<bool> (handle, IPPROTO_IP, IP_MULTICAST_LOOP, enable);
@@ -766,4 +780,71 @@ bool DatagramSocket::setEnablePortReuse (bool enabled)
  #pragma warning (pop)
 #endif
 
+//==============================================================================
+#if JUCE_UNIT_TESTS
+
+struct SocketTests : public UnitTest
+{
+    SocketTests()
+        : UnitTest ("Sockets", "Networking")
+    {
+    }
+
+    void runTest() override
+    {
+        auto localHost = IPAddress::local();
+        int portNum = 12345;
+
+        beginTest ("StreamingSocket");
+        {
+            StreamingSocket socketServer;
+
+            expect (socketServer.isConnected() == false);
+            expect (socketServer.getHostName().isEmpty());
+            expect (socketServer.getBoundPort() == -1);
+            expect (socketServer.getRawSocketHandle() == invalidSocket);
+
+            expect (socketServer.createListener (portNum, localHost.toString()));
+
+            StreamingSocket socket;
+
+            expect (socket.connect (localHost.toString(), portNum));
+
+            expect (socket.isConnected() == true);
+            expect (socket.getHostName() == localHost.toString());
+            expect (socket.getBoundPort() != -1);
+            expect (socket.getRawSocketHandle() != invalidSocket);
+
+            socket.close();
+
+            expect (socket.isConnected() == false);
+            expect (socket.getHostName().isEmpty());
+            expect (socket.getBoundPort() == -1);
+            expect (socket.getRawSocketHandle() == invalidSocket);
+        }
+
+        beginTest ("DatagramSocket");
+        {
+            DatagramSocket socket;
+
+            expect (socket.getBoundPort() == -1);
+            expect (socket.getRawSocketHandle() != invalidSocket);
+
+            expect (socket.bindToPort (portNum, localHost.toString()));
+
+            expect (socket.getBoundPort() == portNum);
+            expect (socket.getRawSocketHandle() != invalidSocket);
+
+            socket.shutdown();
+
+            expect (socket.getBoundPort() == -1);
+            expect (socket.getRawSocketHandle() == invalidSocket);
+        }
+    }
+};
+
+static SocketTests socketTests;
+
+#endif
+
 } // namespace juce

modules/juce_core/network/juce_Socket.h

@@ -55,8 +55,8 @@ public:
     //==============================================================================
     /** Binds the socket to the specified local port.
 
-        @returns    true on success; false may indicate that another socket is already bound
-                    on the same port
+        @returns  true on success; false may indicate that another socket is already bound
+                  on the same port
     */
     bool bindToPort (int localPortNumber);
 
@@ -66,8 +66,9 @@ public:
         as well. This is useful if you would like to bind your socket to a specific network
         adapter. Note that localAddress must be an IP address assigned to one of your
         network address otherwise this function will fail.
-        @returns    true on success; false may indicate that another socket is already bound
-                    on the same port
+
+        @returns  true on success; false may indicate that another socket is already bound
+                  on the same port
         @see bindToPort(int localPortNumber), IPAddress::getAllAddresses
     */
     bool bindToPort (int localPortNumber, const String& localAddress);
@@ -76,7 +77,9 @@ public:
 
         This is useful if you need to know to which port the OS has actually bound your
         socket when calling the constructor or bindToPort with zero as the
-        localPortNumber argument. Returns -1 if the function fails.
+        localPortNumber argument.
+
+        @returns  -1 if the function fails
     */
     int getBoundPort() const noexcept;
 
@@ -85,7 +88,7 @@ public:
         If timeOutMillisecs is 0, then this method will block until the operating system
         rejects the connection (which could take a long time).
 
-        @returns true if it succeeds.
+        @returns  true if it succeeds, false if otherwise
         @see isConnected
     */
     bool connect (const String& remoteHostname,
@@ -119,8 +122,8 @@ public:
         If the timeout is < 0, it will wait forever, or else will give up after
         the specified time.
 
-        If the socket is ready on return, this returns 1. If it times-out before
-        the socket becomes ready, it returns 0. If an error occurs, it returns -1.
+        @returns  1 if the socket is ready on return, 0 if it times-out before
+                  the socket becomes ready, or -1 if an error occurs
     */
     int waitUntilReady (bool readyForReading, int timeoutMsecs);
 
@@ -131,7 +134,7 @@ public:
         flag is false, the method will return as much data as is currently available
         without blocking.
 
-        @returns the number of bytes read, or -1 if there was an error.
+        @returns  the number of bytes read, or -1 if there was an error
         @see waitUntilReady
     */
     int read (void* destBuffer, int maxBytesToRead,
@@ -142,7 +145,7 @@ public:
         Note that this method will block unless you have checked the socket is ready
         for writing before calling it (see the waitUntilReady() method).
 
-        @returns the number of bytes written, or -1 if there was an error.
+        @returns  the number of bytes written, or -1 if there was an error
     */
     int write (const void* sourceBuffer, int numBytesToWrite);
 
@@ -156,8 +159,8 @@ public:
         @param portNumber       the port number to listen on
         @param localHostName    the interface address to listen on - pass an empty
                                 string to listen on all addresses
-        @returns    true if it manages to open the socket successfully.
 
+        @returns  true if it manages to open the socket successfully
         @see waitForNextConnection
     */
     bool createListener (int portNumber, const String& localHostName = String());
@@ -202,8 +205,7 @@ class JUCE_API  DatagramSocket  final
 {
 public:
     //==============================================================================
-    /**
-        Creates a datagram socket.
+    /** Creates a datagram socket.
 
         You first need to bind this socket to a port with bindToPort if you intend to read
         from this socket.
@@ -223,8 +225,8 @@ public:
         The localPortNumber is the port on which to bind this socket. If this value is 0,
         the port number is assigned by the operating system.
 
-        @returns    true on success; false may indicate that another socket is already bound
-                    on the same port
+        @returns  true on success; false may indicate that another socket is already bound
+                  on the same port
     */
     bool bindToPort (int localPortNumber);
 
@@ -234,8 +236,9 @@ public:
         as well. This is useful if you would like to bind your socket to a specific network
         adapter. Note that localAddress must be an IP address assigned to one of your
         network address otherwise this function will fail.
-        @returns    true on success; false may indicate that another socket is already bound
-                    on the same port
+
+        @returns  true on success; false may indicate that another socket is already bound
+                  on the same port
         @see bindToPort(int localPortNumber), IPAddress::getAllAddresses
     */
     bool bindToPort (int localPortNumber, const String& localAddress);
@@ -245,7 +248,8 @@ public:
         This is useful if you need to know to which port the OS has actually bound your
         socket when bindToPort was called with zero.
 
-        Returns -1 if the socket didn't bind to any port yet or an error occurred. */
+        @returns  -1 if the socket didn't bind to any port yet or an error occurred
+    */
     int getBoundPort() const noexcept;
 
     /** Returns the OS's socket handle that's currently open. */
@@ -260,8 +264,8 @@ public:
         If the timeout is < 0, it will wait forever, or else will give up after
         the specified time.
 
-        If the socket is ready on return, this returns 1. If it times-out before
-        the socket becomes ready, it returns 0. If an error occurs, it returns -1.
+        @returns  1 if the socket is ready on return, 0 if it times-out before the
+                  socket becomes ready, or -1 if an error occurs
     */
     int waitUntilReady (bool readyForReading, int timeoutMsecs);
 
@@ -272,7 +276,7 @@ public:
         flag is false, the method will return as much data as is currently available
         without blocking.
 
-        @returns the number of bytes read, or -1 if there was an error.
+        @returns  the number of bytes read, or -1 if there was an error
         @see waitUntilReady
     */
     int read (void* destBuffer, int maxBytesToRead,
@@ -285,8 +289,8 @@ public:
         flag is false, the method will return as much data as is currently available
         without blocking.
 
-        @returns the number of bytes read, or -1 if there was an error. On a successful
-                 result, the senderIPAddress value will be set to the IP of the sender.
+        @returns  the number of bytes read, or -1 if there was an error. On a successful
+                  result, the senderIPAddress value will be set to the IP of the sender
         @see waitUntilReady
     */
     int read (void* destBuffer, int maxBytesToRead,
@@ -298,7 +302,7 @@ public:
         Note that this method will block unless you have checked the socket is ready
         for writing before calling it (see the waitUntilReady() method).
 
-        @returns the number of bytes written, or -1 if there was an error.
+        @returns  the number of bytes written, or -1 if there was an error
     */
     int write (const String& remoteHostname, int remotePortNumber,
                const void* sourceBuffer, int numBytesToWrite);
@@ -306,8 +310,10 @@ public:
     /** Closes the underlying socket object.
 
         Closes the underlying socket object and aborts any read or write operations.
-        Note that all other methods will return an error after this call. This
-        method is useful if another thread is blocking in a read/write call and you
+        Note that all other methods will return an error after this call and the object
+        cannot be re-used.
+
+        This method is useful if another thread is blocking in a read/write call and you
         would like to abort the read/write thread. Simply deleting the socket
         object without calling shutdown may cause a race-condition where the read/write
         returns just before the socket is deleted and the subsequent read/write would
@@ -319,17 +325,20 @@ public:
 
     //==============================================================================
     /** Join a multicast group.
-        @returns true if it succeeds.
+
+        @returns  true if it succeeds
     */
     bool joinMulticast (const String& multicastIPAddress);
 
     /** Leave a multicast group.
-        @returns true if it succeeds.
+
+        @returns  true if it succeeds
     */
     bool leaveMulticast (const String& multicastIPAddress);
 
     /** Enables or disables multicast loopback.
-        @returns true if it succeeds.
+
+        @returns  true if it succeeds
     */
     bool setMulticastLoopbackEnabled (bool enableLoopback);
 
@@ -340,7 +349,7 @@ public:
         Do not use this if your socket handles sensitive data as it could be
         read by any, possibly malicious, third-party apps.
 
-        Returns true on success.
+        @returns  true on success
     */
     bool setEnablePortReuse (bool enabled);