diag: say why a close failed, and total what the program thinks it holds (#98)

The 1.2.11 accounting answered 'which site' and stopped. On a node with 3h51m of uptime it reported 58 live sockets while Windows attributed 112 to the process, 47 of them bound and never connected, against 55 closes that had returned an error -- with nothing recording what those errors were. Close failures are now counted per code per site, and the table totals, so the live figure can be compared directly against what the OS attributes to the process. Instrumentation only.

2 files changed, 52 insertions, 0 deletions

diff --git a/src/net.cpp b/src/net.cpp
index 7a13257..4fe5f15 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -66,8 +66,12 @@ static string SockAccountingText()
 
     string str = "\n  sockets by where they were created\n";
     str += "  site                       opened   closed     live  close failed\n";
+    long long nTotalOpened = 0, nTotalClosed = 0, nTotalFailed = 0;
     for (int i = 0; i < SOCK_SITES; i++)
     {
+        nTotalOpened += nOpened[i];
+        nTotalClosed += nClosed[i];
+        nTotalFailed += nFailed[i];
         if (nOpened[i] == 0)
             continue;
         str += strprintf("  %-24s %8lld %8lld %8lld %13lld\n", pszSockSite[i],
@@ -75,6 +79,31 @@ static string SockAccountingText()
     }
     str += strprintf("  %-24s %8s %8lld %8s %13s\n", "closed but never tagged",
                      "-", nUntagged, "-", "-");
+    str += strprintf("  %-24s %8lld %8lld %8lld %13lld\n", "TOTAL",
+                     nTotalOpened, nTotalClosed, nTotalOpened - nTotalClosed, nTotalFailed);
+
+    // The live total above is what this program believes it is holding. Compare
+    // it against what the operating system attributes to the process: on
+    // Windows, `Get-NetTCPConnection | ? OwningProcess -eq <pid>`. A gap there
+    // is the leak, and the codes below say what the failed closes hit -- 10038
+    // (WSAENOTSOCK) means the handle was already gone and this close was a
+    // second one, which is a different bug from a descriptor that stayed.
+    if (nTotalFailed > 0)
+    {
+        str += "  close failures by code\n";
+        for (int i = 0; i < SOCK_SITES; i++)
+        {
+            if (nFailed[i] == 0)
+                continue;
+            map<int, long long> mapErr;
+            BtfSockCloseErrors(i, mapErr);
+            string strCodes;
+            for (map<int, long long>::const_iterator mi = mapErr.begin(); mi != mapErr.end(); ++mi)
+                strCodes += strprintf("%s%d x%lld", strCodes.empty() ? "" : ", ",
+                                      mi->first, mi->second);
+            str += strprintf("  %-24s %s\n", pszSockSite[i], strCodes.c_str());
+        }
+    }
     return str;
 }
 
diff --git a/src/sockcount.h b/src/sockcount.h
index 19f0827..a42abb9 100644
--- a/src/sockcount.h
+++ b/src/sockcount.h
@@ -23,6 +23,7 @@
 #ifndef BITFLASH_SOCKCOUNT_H
 #define BITFLASH_SOCKCOUNT_H
 
+#include <cerrno>
 #include <map>
 #include <mutex>
 
@@ -52,6 +53,13 @@ struct BtfSockAccount
     long long nOpened[SOCK_SITES];
     long long nClosed[SOCK_SITES];
     long long nCloseFailed[SOCK_SITES];
+    // Why the close failed, counted per code per site. Measured on a node with
+    // 3h51m of uptime: the program believed it held 58 sockets and Windows
+    // attributed 112 to the process, 47 of them bound to an ephemeral port and
+    // never connected -- against 55 closes that had returned an error. The
+    // shapes match, and the error code is the piece that says whether the
+    // descriptor is still there or was already gone.
+    std::map<int, long long> mapCloseErr[SOCK_SITES];
     long long nClosedUntagged;
     BtfSockAccount() : nClosedUntagged(0)
     {
@@ -102,8 +110,10 @@ inline int BtfCloseSocket(SOCKET hSocket)
     }
 #ifdef _WIN32
     int nRet = ::closesocket(hSocket);
+    int nErr = (nRet != 0) ? WSAGetLastError() : 0;
 #else
     int nRet = ::close(hSocket);
+    int nErr = (nRet != 0) ? errno : 0;
 #endif
     // A close that fails leaves the descriptor -- and on Windows the ephemeral
     // port it holds -- in place. Counted because a socket the program believes
@@ -115,6 +125,7 @@ inline int BtfCloseSocket(SOCKET hSocket)
         BtfSockAccount& a = BtfSockAccounting();
         std::lock_guard<std::mutex> lock(a.mtx);
         a.nCloseFailed[nSite]++;
+        a.mapCloseErr[nSite][nErr]++;
     }
     return nRet;
 }
@@ -134,4 +145,16 @@ inline void BtfSockSnapshot(long long* pOpened, long long* pClosed, long long* p
     *pUntagged = a.nClosedUntagged;
 }
 
+// The close-failure codes for one site, newest counts included. Separate call
+// because the table above is fixed-width and this is a list of unknown length.
+inline void BtfSockCloseErrors(int nSite, std::map<int, long long>& mapOut)
+{
+    mapOut.clear();
+    if (nSite < 0 || nSite >= SOCK_SITES)
+        return;
+    BtfSockAccount& a = BtfSockAccounting();
+    std::lock_guard<std::mutex> lock(a.mtx);
+    mapOut = a.mapCloseErr[nSite];
+}
+
 #endif