Skip to content

Commit d8edfb0

Browse files
gophergogogophergogo
authored andcommitted
Count TCP server connections in the public stats (#219)
server_stats_.connections_active and connections_total are supposed to track accepted connections on the server, but ConnectionImpl only raises ConnectionEvent::Connected on the client-side connect-completion path, never for server-accepted sockets. The per-connection lifecycle adapter therefore never fired the Connected branch of onConnectionLifecycleEvent, and the counters stayed at 0 on connect. The matching decrement on RemoteClose/LocalClose still ran, so after the first client disconnect connections_active wrapped the uint64 to UINT64_MAX. Bump the counters directly in onNewConnection, which is the server-side checkpoint where the socket is accepted, the filter chain is built, and the call runs on the dispatcher thread. The symmetric decrement in onConnectionLifecycleEvent also runs on that thread, so the stats stay consistent without extra synchronization. Remove the dead Connected/ConnectedZeroRtt increment branch from onConnectionLifecycleEvent to keep one canonical increment site -- otherwise a future change that did raise Connected for server connections would quietly double-count. The stdio path is unaffected: McpConnectionManager still raises Connected through ServerProtocolCallbacks::onConnectionEvent, which routes to the separate McpServer::onConnectionEvent handler and keeps its own increment.
1 parent af8ca80 commit d8edfb0

1 file changed

Lines changed: 20 additions & 3 deletions

File tree

src/server/mcp_server.cc

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -818,11 +818,16 @@ void McpServer::onConnectionLifecycleEvent(network::Connection* connection,
818818
assert(main_dispatcher_ && main_dispatcher_->isThreadSafe() &&
819819
"onConnectionLifecycleEvent off dispatcher");
820820
// Runs in dispatcher thread via the per-connection adapter.
821+
//
822+
// Only close events reach us here in practice. ConnectionImpl does not
823+
// raise Connected for server-accepted sockets (it is raised only from
824+
// the client-side connect-completion path), so a Connected case would
825+
// be dead code. The matching connections_total/connections_active
826+
// increment lives in onNewConnection, which is the server-side
827+
// equivalent checkpoint.
821828
switch (event) {
822829
case network::ConnectionEvent::Connected:
823830
case network::ConnectionEvent::ConnectedZeroRtt:
824-
server_stats_.connections_total++;
825-
server_stats_.connections_active++;
826831
return;
827832
case network::ConnectionEvent::RemoteClose:
828833
case network::ConnectionEvent::LocalClose:
@@ -1312,8 +1317,20 @@ void McpServer::onNewConnection(network::ConnectionPtr&& connection) {
13121317
// Set current connection for request processing context
13131318
current_connection_ = conn_ptr;
13141319

1315-
// Update connection count
1320+
// Update connection count.
1321+
//
1322+
// We bump the public stats here rather than wait for a Connected event
1323+
// via the per-connection lifecycle adapter, because ConnectionImpl
1324+
// never raises Connected for server-accepted sockets -- the event is
1325+
// only raised on the client-side connect-completion path. onNewConnection
1326+
// is the equivalent checkpoint for server: the socket is already
1327+
// accepted and the filter chain is built by the time we run here, and
1328+
// this call is on the dispatcher thread, so the atomic increments land
1329+
// on the same thread that the decrement in onConnectionLifecycleEvent
1330+
// runs on.
13161331
++num_connections_;
1332+
server_stats_.connections_total++;
1333+
server_stats_.connections_active++;
13171334

13181335
// Store the connection to keep it alive
13191336
// Following production pattern: server owns connections in dispatcher thread

0 commit comments

Comments
 (0)