wsd: shutdown and close sockets early

We now close socket FDs when we shutdown
the underlying connection, rather than
waiting until the Socket object is destroyed.

This fixes a recent regression where we leaked
FDs because we invalidated before closing.
This was introduced in af747c1.

In addition, we unify the logic to shutdown
and close and localize all in setClosed().

Change-Id: Ib3cb84394fca53f41a64aba9201c5779ee536dcf
Signed-off-by: Ashod Nakashian <[email protected]>

net/Socket.hpp

@@ -159,16 +159,7 @@ class Socket
     {
         LOG_TRC("Socket dtor");
 
-        // Doesn't block on sockets; no error handling needed.
-        if constexpr (!Util::isMobileApp())
-        {
-            ::close(_fd);
-            LOG_DBG("Closed socket " << toStringImpl());
-        }
-        else
-        {
-            fakeSocketClose(_fd);
-        }
+        setClosed();
     }
 
     /// Returns true iff this socket has been closed or is invalid.
@@ -217,15 +208,7 @@ class Socket
     /// TODO: Support separate read/write shutdown.
     virtual void shutdown()
     {
-        if (!_noShutdown && !isClosed())
-        {
-            LOG_TRC("Socket shutdown RDWR. " << *this);
-            if constexpr (!Util::isMobileApp())
-                ::shutdown(_fd, SHUT_RDWR);
-            else
-                fakeSocketShutdown(_fd);
-            setClosed(); // Invalidate the FD.
-        }
+        setClosed(); // Shutdown and close if not closed.
     }
 
     /// Prepare our poll record; adjust @timeoutMaxMs downwards
@@ -435,18 +418,42 @@ class Socket
     /// avoid doing a shutdown before close
     void setNoShutdown() { _noShutdown = true; }
 
-    /// Explicitly marks this socket closed, i.e. rejected from polling and potentially shutdown
+    /// Shutdown (if not flagged with no-shutdown) and close the socket.
     /// Note: to preserve the original FD post closing (f.e. in logs and debugger), we negate it.
     void setClosed()
     {
-        if (_fd > 0)
-            _fd = -_fd;
-        else if (_fd == 0) // Unlikely, but technically possible.
-            _fd = -1;
-    }
+        if (!isClosed())
+        {
+            // Copy to invalidate immediately as fakeSocket can throw.
+            const int fd = _fd;
 
-    /// Explicitly marks this socket and the given SocketDisposition closed
-    void setClosed(SocketDisposition &disposition) { setClosed(); disposition.setClosed(); }
+            // Invalidate the FD by negating to preserve the original value.
+            if (_fd > 0)
+                _fd = -_fd;
+            else if (_fd == 0) // Unlikely, but technically possible.
+                _fd = -1;
+
+            if (!_noShutdown)
+            {
+                LOG_TRC("Socket shutdown RDWR. " << *this);
+                if constexpr (!Util::isMobileApp())
+                    ::shutdown(fd, SHUT_RDWR);
+                else
+                    fakeSocketShutdown(fd);
+            }
+
+            // Doesn't block on sockets; no error handling needed.
+            if constexpr (!Util::isMobileApp())
+            {
+                ::close(fd);
+                LOG_DBG("Closed socket " << toStringImpl());
+            }
+            else
+            {
+                fakeSocketClose(fd);
+            }
+        }
+    }
 
 private:
     /// Create socket of the given type.
@@ -1583,7 +1590,8 @@ class StreamSocket : public Socket,
         {
             LOG_TRC("Closed. Firing onDisconnect.");
             _socketHandler->onDisconnect();
-            setClosed(disposition);
+            setClosed();
+            disposition.setClosed();
         }
         else if (isClosed())
             disposition.setClosed();