diff --git a/src/nxt_conn.h b/src/nxt_conn.h index 5717d3c99..7dcc268dc 100644 --- a/src/nxt_conn.h +++ b/src/nxt_conn.h @@ -160,7 +160,23 @@ struct nxt_conn_s { uint8_t block_read; /* 1 bit */ uint8_t block_write; /* 1 bit */ uint8_t delayed; /* 1 bit */ - uint8_t idle; /* 1 bit */ + +#define NXT_CONN_TRACK_NONE 0 +#define NXT_CONN_TRACK_IDLE 1 +#define NXT_CONN_TRACK_ACTIVE 2 + + /* + * Tri-state membership in the engine's idle/active connection queues. + * NONE (0): conn was never placed onto an engine tracking queue + * (e.g. controller clients, proxy peers). c->link is owned + * by some other subsystem. + * IDLE (1): conn is on engine->idle_connections. + * ACTIVE (2): conn is on engine->active_connections. + * + * Set/cleared exclusively by nxt_conn_idle() / nxt_conn_active() and + * cleared by nxt_conn_close() when the conn is unlinked from its queue. + */ + uint8_t idle; /* 2 bits */ #define NXT_CONN_SENDFILE_OFF 0 #define NXT_CONN_SENDFILE_ON 1 @@ -295,24 +311,62 @@ NXT_EXPORT void nxt_event_conn_job_sendfile(nxt_task_t *task, } while (0) +/* + * Place conn onto engine->idle_connections. + * + * Valid prior states: + * NXT_CONN_TRACK_NONE - freshly created conn (e.g. just-accepted in + * nxt_conn_accept()). Link is not on any queue. + * NXT_CONN_TRACK_ACTIVE - keep-alive transition: conn is currently on + * engine->active_connections and is being moved + * back to idle awaiting the next request. + * + * After this macro: conn is on idle_connections, c->idle == TRACK_IDLE. + */ #define nxt_conn_idle(engine, c) \ do { \ nxt_event_engine_t *e = engine; \ \ + if (c->idle == NXT_CONN_TRACK_ACTIVE) { \ + nxt_queue_remove(&c->link); \ + nxt_atomic_fetch_add(&e->active_conns_cnt, -1); \ + } \ + \ nxt_queue_insert_head(&e->idle_connections, &c->link); \ \ - c->idle = 1; \ + c->idle = NXT_CONN_TRACK_IDLE; \ e->idle_conns_cnt++; \ } while (0) +/* + * Move conn onto engine->active_connections. + * + * Valid prior states: + * NXT_CONN_TRACK_IDLE - conn is currently on engine->idle_connections; + * a request has begun arriving (or idle timeout + * is firing a forced response). + * + * After this macro: conn is on active_connections, c->idle == TRACK_ACTIVE. + * + * Note: every existing call site in nxt_h1proto.c invokes this only on a + * conn that just transitioned from idle (handlers wired to *_idle_state / + * *_keepalive_state). The TRACK_IDLE precondition is not asserted to keep + * the macro branch-free in the hot path; if a future caller violates it, + * idle_conns_cnt will go negative and the queue will corrupt -- both + * detectable under ASan via the ASSERT in nxt_conn_close(). + */ #define nxt_conn_active(engine, c) \ do { \ nxt_event_engine_t *e = engine; \ \ nxt_queue_remove(&c->link); \ + e->idle_conns_cnt -= (c->idle == NXT_CONN_TRACK_IDLE); \ + \ + nxt_queue_insert_head(&e->active_connections, &c->link); \ \ - e->idle_conns_cnt -= c->idle; \ + c->idle = NXT_CONN_TRACK_ACTIVE; \ + nxt_atomic_fetch_add(&e->active_conns_cnt, 1); \ } while (0) diff --git a/src/nxt_conn_close.c b/src/nxt_conn_close.c index bdd669515..aba4de67f 100644 --- a/src/nxt_conn_close.c +++ b/src/nxt_conn_close.c @@ -119,7 +119,31 @@ nxt_conn_close_handler(nxt_task_t *task, void *obj, void *data) nxt_socket_close(task, c->socket.fd); c->socket.fd = -1; - if (c->idle) { + /* + * Unlink from the engine idle/active tracking queue, if any. + * Conns that were never tracked (e.g. controller clients, proxy + * peers) keep c->idle == NXT_CONN_TRACK_NONE and own c->link + * separately, so this branch must not run for them. + * + * closed_conns_cnt is incremented symmetrically for both idle and + * active closes (P4.5). Prior to P4.5 only idle closes were + * counted, but in practice every conn passing through accept was + * placed onto the idle queue and c->idle was never reset to 0 by + * nxt_conn_active(), so the counter was already incrementing for + * effectively every accepted-conn close. The new symmetric form + * preserves that behaviour while also covering the case where a + * conn closes while genuinely active. + */ + if (c->idle == NXT_CONN_TRACK_IDLE) { + nxt_queue_remove(&c->link); + engine->idle_conns_cnt--; + c->idle = NXT_CONN_TRACK_NONE; + engine->closed_conns_cnt++; + + } else if (c->idle == NXT_CONN_TRACK_ACTIVE) { + nxt_queue_remove(&c->link); + nxt_atomic_fetch_add(&engine->active_conns_cnt, -1); + c->idle = NXT_CONN_TRACK_NONE; engine->closed_conns_cnt++; } @@ -157,7 +181,17 @@ nxt_conn_close_timer_handler(nxt_task_t *task, void *obj, void *data) nxt_socket_close(task, c->socket.fd); c->socket.fd = -1; - if (c->idle) { + /* See nxt_conn_close_handler() for the symmetry rationale (P4.5). */ + if (c->idle == NXT_CONN_TRACK_IDLE) { + nxt_queue_remove(&c->link); + engine->idle_conns_cnt--; + c->idle = NXT_CONN_TRACK_NONE; + engine->closed_conns_cnt++; + + } else if (c->idle == NXT_CONN_TRACK_ACTIVE) { + nxt_queue_remove(&c->link); + nxt_atomic_fetch_add(&engine->active_conns_cnt, -1); + c->idle = NXT_CONN_TRACK_NONE; engine->closed_conns_cnt++; } } diff --git a/src/nxt_event_engine.c b/src/nxt_event_engine.c index 78c79bb1a..c3fa904eb 100644 --- a/src/nxt_event_engine.c +++ b/src/nxt_event_engine.c @@ -139,6 +139,9 @@ nxt_event_engine_create(nxt_task_t *task, nxt_queue_init(&engine->joints); nxt_queue_init(&engine->listen_connections); nxt_queue_init(&engine->idle_connections); + nxt_queue_init(&engine->active_connections); + + engine->active_conns_cnt = 0; return engine; diff --git a/src/nxt_event_engine.h b/src/nxt_event_engine.h index 291ea749b..8f5666851 100644 --- a/src/nxt_event_engine.h +++ b/src/nxt_event_engine.h @@ -481,10 +481,12 @@ struct nxt_event_engine_s { nxt_queue_t joints; nxt_queue_t listen_connections; nxt_queue_t idle_connections; + nxt_queue_t active_connections; nxt_array_t *mem_cache; nxt_atomic_uint_t accepted_conns_cnt; nxt_atomic_uint_t idle_conns_cnt; + nxt_atomic_uint_t active_conns_cnt; nxt_atomic_uint_t closed_conns_cnt; nxt_atomic_uint_t requests_cnt; diff --git a/src/nxt_runtime.c b/src/nxt_runtime.c index de76f19e0..a7f2111cf 100644 --- a/src/nxt_runtime.c +++ b/src/nxt_runtime.c @@ -484,7 +484,13 @@ nxt_runtime_close_idle_connections(nxt_event_engine_t *engine) c = nxt_queue_link_data(link, nxt_conn_t, link); if (!c->socket.read_ready) { - nxt_queue_remove(link); + /* + * Do not unlink here: nxt_conn_close() now removes the conn + * from idle_connections in its async close handler and + * decrements idle_conns_cnt accordingly (P4.5). Iteration is + * safe because `next` was captured before close was scheduled + * and the actual unlink runs from a later work-queue tick. + */ nxt_conn_close(engine, c); } }