Skip to content

Commit 9574867

Browse files
committed
MINOR: muxes: enforce thread-safety for private idle conns
When a backend connnection becomes idle, muxes must activate some protection to mark future access on it as dangerous. Indeed, once a connection is inserted in an idle list, it may be manipulated by another thread, either via takeover or scheduled for purging. Private idle connections are stored into a session instead of the server tree. They are never subject to a takeover for reuse or purge mechanism. As such, currently they do not require the same level of protection. However, a new patch will introduce support for private idle connections purging. Thus, the purpose of this patch is to ensure protection is activated as well now. TASK_F_USR1 was already set on them as an anticipation for such need. Only some extra operations were missing, most notably xprt_set_idle() invokation. Also, return path of muxes detach operation is adjusted to ensure such connection are never accessed after insertion.
1 parent b18b5e2 commit 9574867

File tree

4 files changed

+32
-0
lines changed

4 files changed

+32
-0
lines changed

src/mux_fcgi.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3748,13 +3748,21 @@ static void fcgi_detach(struct sedesc *sd)
37483748
* that the handler needs to check it under the idle conns lock.
37493749
*/
37503750
HA_ATOMIC_OR(&fconn->wait_event.tasklet->state, TASK_F_USR1);
3751+
xprt_set_idle(fconn->conn, fconn->conn->xprt, fconn->conn->xprt_ctx);
37513752

37523753
/* Ensure session can keep a new idle connection. */
37533754
if (session_check_idle_conn(sess, fconn->conn) != 0) {
37543755
fconn->conn->mux->destroy(fconn);
37553756
TRACE_DEVEL("outgoing connection killed", FCGI_EV_STRM_END|FCGI_EV_FCONN_ERR);
37563757
return;
37573758
}
3759+
3760+
/* At this point, the connection is inserted into
3761+
* session list and marked as idle, so it may already
3762+
* have been purged from another thread.
3763+
*/
3764+
TRACE_DEVEL("private connection marked as idle", FCGI_EV_STRM_END, fconn->conn);
3765+
return;
37583766
}
37593767
}
37603768
else {

src/mux_h1.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1151,13 +1151,21 @@ static int h1s_finish_detach(struct h1s *h1s)
11511151
* that the handler needs to check it under the idle conns lock.
11521152
*/
11531153
HA_ATOMIC_OR(&h1c->wait_event.tasklet->state, TASK_F_USR1);
1154+
h1c->conn->xprt->subscribe(h1c->conn, h1c->conn->xprt_ctx, SUB_RETRY_RECV, &h1c->wait_event);
1155+
xprt_set_idle(h1c->conn, h1c->conn->xprt, h1c->conn->xprt_ctx);
11541156

11551157
/* Ensure session can keep a new idle connection. */
11561158
if (session_check_idle_conn(sess, h1c->conn)) {
11571159
TRACE_DEVEL("outgoing connection rejected", H1_EV_STRM_END|H1_EV_H1C_END, h1c->conn);
11581160
h1c->conn->mux->destroy(h1c);
11591161
goto released;
11601162
}
1163+
1164+
/* At this point, the connection is inserted into
1165+
* session list and marked as idle, so it may already
1166+
* have been purged from another thread.
1167+
*/
1168+
goto end;
11611169
}
11621170
else {
11631171
if (h1c->conn->owner == sess)

src/mux_h2.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5558,13 +5558,21 @@ static void h2_detach(struct sedesc *sd)
55585558
* that the handler needs to check it under the idle conns lock.
55595559
*/
55605560
HA_ATOMIC_OR(&h2c->wait_event.tasklet->state, TASK_F_USR1);
5561+
xprt_set_idle(h2c->conn, h2c->conn->xprt, h2c->conn->xprt_ctx);
55615562

55625563
/* Ensure session can keep a new idle connection. */
55635564
if (session_check_idle_conn(sess, h2c->conn) != 0) {
55645565
h2c->conn->mux->destroy(h2c);
55655566
TRACE_DEVEL("leaving without reusable idle connection", H2_EV_STRM_END);
55665567
return;
55675568
}
5569+
5570+
/* At this point, the connection is inserted into
5571+
* session list and marked as idle, so it may already
5572+
* have been purged from another thread.
5573+
*/
5574+
TRACE_DEVEL("private connection marked as idle", H2_EV_STRM_END);
5575+
return;
55685576
}
55695577
}
55705578
else {

src/mux_spop.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3005,13 +3005,21 @@ static void spop_detach(struct sedesc *sd)
30053005
* that the handler needs to check it under the idle conns lock.
30063006
*/
30073007
HA_ATOMIC_OR(&spop_conn->wait_event.tasklet->state, TASK_F_USR1);
3008+
xprt_set_idle(spop_conn->conn, spop_conn->conn->xprt, spop_conn->conn->xprt_ctx);
30083009

30093010
/* Ensure session can keep a new idle connection. */
30103011
if (session_check_idle_conn(sess, spop_conn->conn) != 0) {
30113012
spop_conn->conn->mux->destroy(spop_conn);
30123013
TRACE_DEVEL("leaving without reusable idle connection", SPOP_EV_STRM_END);
30133014
return;
30143015
}
3016+
3017+
/* At this point, the connection is inserted into
3018+
* session list and marked as idle, so it may already
3019+
* have been purged from another thread.
3020+
*/
3021+
TRACE_DEVEL("private connection marked as idle", SPOP_EV_STRM_END);
3022+
return;
30153023
}
30163024
}
30173025
else {

0 commit comments

Comments
 (0)