Repository: bitcoin/bitcoin
File: src/net.cpp (C++)

Code before:

void CConnman::DisconnectNodes()
{
    AssertLockNotHeld(m_nodes_mutex);
    AssertLockNotHeld(m_reconnections_mutex);

    // Use a temporary variable to accumulate desired reconnections, so we don't need
    // m_reconnections_mutex while holding m_nodes_mutex.
    decltype(m_reconnections) reconnections_to_add;

    {
        LOCK(m_nodes_mutex);

        if (!fNetworkActive) {
            // Disconnect any connected nodes
            for (CNode* pnode : m_nodes) {
                if (!pnode->fDisconnect) {
                    LogDebug(BCLog::NET, "Network not active, dropping peer=%d\n", pnode->GetId());
                    pnode->fDisconnect = true;
                }
            }
        }

        // Disconnect unused nodes
        std::vector<CNode*> nodes_copy = m_nodes;
        for (CNode* pnode : nodes_copy)
        {
            if (pnode->fDisconnect)
            {
                // remove from m_nodes
                m_nodes.erase(remove(m_nodes.begin(), m_nodes.end(), pnode), m_nodes.end());

                // Add to reconnection list if appropriate. We don't reconnect right here, because
                // the creation of a connection is a blocking operation (up to several seconds),
                // and we don't want to hold up the socket handler thread for that long.
                if (pnode->m_transport->ShouldReconnectV1()) {
                    reconnections_to_add.push_back({
                        .addr_connect = pnode->addr,
                        .grant = std::move(pnode->grantOutbound),
                        .destination = pnode->m_dest,
                        .conn_type = pnode->m_conn_type,
                        .use_v2transport = false});
                    LogDebug(BCLog::NET, "retrying with v1 transport protocol for peer=%d\n", pnode->GetId());
                }

                // release outbound grant (if any)
                pnode->grantOutbound.Release();

                // close socket and cleanup
                pnode->CloseSocketDisconnect();


Review comment (suggestion):
_(suggestion at random place in the code)_

Consider to also adjust the `CConnman::DisconnectNode()` functions.


Code after:

    // Max connections of specified type already exist
    if (max_connections != std::nullopt && existing_connections >= max_connections) return false;

    // Max total outbound connections already exist
    CSemaphoreGrant grant(*semOutbound, true);
    if (!grant) return false;

    OpenNetworkConnection(CAddress(), false, std::move(grant), address.c_str(), conn_type, /*use_v2transport=*/use_v2transport);
    return true;
}

void CConnman::DisconnectNodes()
{
    AssertLockNotHeld(m_nodes_mutex);
    AssertLockNotHeld(m_reconnections_mutex);

    // Use a temporary variable to accumulate desired reconnections, so we don't need
    // m_reconnections_mutex while holding m_nodes_mutex.
    decltype(m_reconnections) reconnections_to_add;

    {
        LOCK(m_nodes_mutex);

        if (!fNetworkActive) {
            // Disconnect any connected nodes
            for (CNode* pnode : m_nodes) {
                if (!pnode->fDisconnect) {
                    LogDebug(BCLog::NET, "Network not active, %s\n", pnode->DisconnectMsg(fLogIPs));
                    pnode->fDisconnect = true;
                }
            }
        }

        // Disconnect unused nodes
        std::vector<CNode*> nodes_copy = m_nodes;
        for (CNode* pnode : nodes_copy)
        {
            if (pnode->fDisconnect)
            {
                // remove from m_nodes
                m_nodes.erase(remove(m_nodes.begin(), m_nodes.end(), pnode), m_nodes.end());

                // Add to reconnection list if appropriate. We don't reconnect right here, because
                // the creation of a connection is a blocking operation (up to several seconds),
                // and we don't want to hold up the socket handler thread for that long.
                if (pnode->m_transport->ShouldReconnectV1()) {
                    reconnections_to_add.push_back({
                        .addr_connect = pnode->addr,
                        .grant = std::move(pnode->grantOutbound),
                        .destination = pnode->m_dest,