mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-11-29 23:01:03 +01:00
Code Issues Projects Releases Wiki Activity

MEDIUM: sink: assume sft appctx stickiness

Browse Source 
As mentioned in b40d804 ("MINOR: sink: add some comments about sft->appctx
usage in applet handlers"), there are few places in the code where it
looks like we assumed that the applet callbacks such as
sink_forward_session_init() or sink_forward_io_handler() could be
executing an appctx whose sft is detached from the appctx
(appctx != sft->appctx).

In practise this should not be happening since an appctx sticks to the
same thread its entire lifetime, and the only times sft->appctx is
effectively assigned is during the session/appctx creation (in
process_sink_forward()) or release.

Thus if sft->appctx wouldn't point to the appctx that the sft was bound
to after appctx creation, it would probably indicate a bug rather than
an expected condition. To further emphasize that and prevent the
confusion, and since 3.1-dev4 was released, let's remove such checks and
instead add a BUG_ON to ensure this never happens.

In _sink_forward_io_handler(), the "hard_close" label was removed since
there are no more uses for it (no hard errors may be caught from the
function for now)
This commit is contained in:
Aurelien DARRAGON 2024-07-24 15:57:04 +02:00
parent 28cb01f8e8
commit e328056ddc
1 changed files with 18 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
src/sink.c
View File

@ -375,11 +375,7 @@ static void _sink_forward_io_handler(struct appctx *appctx,
} }
HA_SPIN_LOCK(SFT_LOCK, &sft->lock); HA_SPIN_LOCK(SFT_LOCK, &sft->lock);
if (appctx != sft->appctx) { BUG_ON(appctx != sft->appctx);
/* FIXME: is this even supposed to happen? */
HA_SPIN_UNLOCK(SFT_LOCK, &sft->lock);
goto hard_close;
}
MT_LIST_DELETE(&appctx->wait_entry); MT_LIST_DELETE(&appctx->wait_entry);
@ -426,10 +422,15 @@ out:
return; return;
soft_close: soft_close:
/* be careful: since the socket lacks the NOLINGER flag (on purpose)
* soft_close will result in the port staying in TIME_WAIT state:
* don't abuse from soft_close!
*/
se_fl_set(appctx->sedesc, SE_FL_EOS|SE_FL_EOI); se_fl_set(appctx->sedesc, SE_FL_EOS|SE_FL_EOI);
return; /* if required, hard_close could be achieve by using SE_FL_EOS|SE_FL_ERROR
hard_close: * flag combination: RST will be sent, TIME_WAIT will be avoided as if
se_fl_set(appctx->sedesc, SE_FL_EOS|SE_FL_ERROR); * we performed a normal close with NOLINGER flag set
*/
} }
/* /*
@ -476,6 +477,8 @@ static int sink_forward_session_init(struct appctx *appctx)
*/ */
HA_SPIN_LOCK(SFT_LOCK, &sft->lock); HA_SPIN_LOCK(SFT_LOCK, &sft->lock);
BUG_ON(sft->appctx != appctx);
if (!sockaddr_alloc(&addr, &sft->srv->addr, sizeof(sft->srv->addr))) if (!sockaddr_alloc(&addr, &sft->srv->addr, sizeof(sft->srv->addr)))
goto out_error; goto out_error;
/* srv port should be learned from srv->svc_port not from srv->addr */ /* srv port should be learned from srv->svc_port not from srv->addr */
@ -496,9 +499,6 @@ static int sink_forward_session_init(struct appctx *appctx)
applet_expect_no_data(appctx); applet_expect_no_data(appctx);
/* FIXME: redundant? was already assigned in process_sink_forward() */
sft->appctx = appctx;
HA_SPIN_UNLOCK(SFT_LOCK, &sft->lock); HA_SPIN_UNLOCK(SFT_LOCK, &sft->lock);
return 0; return 0;
@ -518,9 +518,8 @@ static void sink_forward_session_release(struct appctx *appctx)
return; return;
HA_SPIN_LOCK(SFT_LOCK, &sft->lock); HA_SPIN_LOCK(SFT_LOCK, &sft->lock);
if (sft->appctx == appctx) BUG_ON(sft->appctx != appctx);
__sink_forward_session_deinit(sft); __sink_forward_session_deinit(sft);
/* FIXME: is 'sft->appctx != appctx' even supposed to happen? */
HA_SPIN_UNLOCK(SFT_LOCK, &sft->lock); HA_SPIN_UNLOCK(SFT_LOCK, &sft->lock);
} }
@ -609,7 +608,11 @@ static struct task *process_sink_forward(struct task * task, void *context, unsi
if (!stopping) { if (!stopping) {
while (sft) { while (sft) {
HA_SPIN_LOCK(SFT_LOCK, &sft->lock); HA_SPIN_LOCK(SFT_LOCK, &sft->lock);
/* if appctx is NULL, start a new session */ /* If appctx is NULL, start a new session and perform the appctx
* assigment right away since the applet is not supposed to change
* during the session lifetime. By doing the assignment now we
* make sure to start the session exactly once.
*/
if (!sft->appctx) if (!sft->appctx)
sft->appctx = sink_forward_session_create(sink, sft); sft->appctx = sink_forward_session_create(sink, sft);
HA_SPIN_UNLOCK(SFT_LOCK, &sft->lock); HA_SPIN_UNLOCK(SFT_LOCK, &sft->lock);