Merge bitcoin/bitcoin#23604: Use Sock in CNode

ef5014d256638735b292672c774446db4003f03b style: wrap long lines in CNode creation and add some comments (Vasil Dimov)
b68349164827f14c472201cad54c4e19a3321261 scripted-diff: rename CNode::cs_hSocket to CNode::m_sock_mutex (Vasil Dimov)
c41a1162ac4da437c5d755e8fe2bf636bed22b0f net: use Sock in CNode (Vasil Dimov)
c5dd72e146dd8fa77d29c8689a42322a4d1ec780 fuzz: move FuzzedSock earlier in src/test/fuzz/util.h (Vasil Dimov)

Pull request description:

  _This is a piece of #21878, chopped off to ease review._

  Change `CNode` to use a pointer to `Sock` instead of a bare `SOCKET`.
  This will help mocking / testing / fuzzing more code.

ACKs for top commit:
  jonatack:
    re-ACK ef5014d256638735b292672c774446db4003f03b changes since last review are the removal of an unneeded dtor and the addition of a style commit
  w0xlt:
    reACK ef5014d
  PastaPastaPasta:
    utACK ef5014d256638735b292672c774446db4003f03b, I have reviewed the code, and believe it makes sense to merge
  theStack:
    Cod-review ACK ef5014d256638735b292672c774446db4003f03b

Tree-SHA512: 7f5414dd339cd2f16f7cbdc5fcec238d68b6d50072934aea10b901f409da28ff1ece6db6e899196616aa8127b8b25ab5b86d000bdcee58b4cadd7a3c1cf560c5

1 files changed, 13 insertions, 4 deletions

diff --git a/src/net.h b/src/net.h
index 4301733525..3f4c8e38ec 100644
--- a/src/net.h
+++ b/src/net.h
@@ -402,7 +402,17 @@ public:
 
     NetPermissionFlags m_permissionFlags{NetPermissionFlags::None};
     std::atomic<ServiceFlags> nServices{NODE_NONE};
-    SOCKET hSocket GUARDED_BY(cs_hSocket);
+
+    /**
+     * Socket used for communication with the node.
+     * May not own a Sock object (after `CloseSocketDisconnect()` or during tests).
+     * `shared_ptr` (instead of `unique_ptr`) is used to avoid premature close of
+     * the underlying file descriptor by one thread while another thread is
+     * poll(2)-ing it for activity.
+     * @see https://github.com/bitcoin/bitcoin/issues/21744 for details.
+     */
+    std::shared_ptr<Sock> m_sock GUARDED_BY(m_sock_mutex);
+
     /** Total size of all vSendMsg entries */
     size_t nSendSize GUARDED_BY(cs_vSend){0};
     /** Offset inside the first vSendMsg already sent */
@@ -410,7 +420,7 @@ public:
     uint64_t nSendBytes GUARDED_BY(cs_vSend){0};
     std::deque<std::vector<unsigned char>> vSendMsg GUARDED_BY(cs_vSend);
     Mutex cs_vSend;
-    Mutex cs_hSocket;
+    Mutex m_sock_mutex;
     Mutex cs_vRecv;
 
     RecursiveMutex cs_vProcessMsg;
@@ -578,8 +588,7 @@ public:
      * criterium in CConnman::AttemptToEvictConnection. */
     std::atomic<std::chrono::microseconds> m_min_ping_time{std::chrono::microseconds::max()};
 
-    CNode(NodeId id, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, ConnectionType conn_type_in, bool inbound_onion);
-    ~CNode();
+    CNode(NodeId id, ServiceFlags nLocalServicesIn, std::shared_ptr<Sock> sock, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, ConnectionType conn_type_in, bool inbound_onion);
     CNode(const CNode&) = delete;
     CNode& operator=(const CNode&) = delete;