refactor(engine): track active connections symmetrically with idle (pre-P5)

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)
 
 

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++;
         }
     }

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;
 

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;
 

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);
         }
     }