mirror of
https://git.haproxy.org/git/haproxy.git/
synced 2025-09-22 14:21:25 +02:00
CLEANUP: checks: remove return statements in locked functions
Given that all spinning loops we've had since 1.8-rc1 were caused by unbalanced lock/unlock, let's get rid of all return statements in the locked check functions and only exit via a a single unlock place.
This commit is contained in:
parent
73247e0757
commit
62ac84f843
87
src/checks.c
87
src/checks.c
@ -704,6 +704,9 @@ static void chk_report_conn_err(struct check *check, int errno_bck, int expired)
|
|||||||
* the connection acknowledgement. If the proxy requires L7 health-checks,
|
* the connection acknowledgement. If the proxy requires L7 health-checks,
|
||||||
* it sends the request. In other cases, it calls set_server_check_status()
|
* it sends the request. In other cases, it calls set_server_check_status()
|
||||||
* to set check->status, check->duration and check->result.
|
* to set check->status, check->duration and check->result.
|
||||||
|
*
|
||||||
|
* Please do NOT place any return statement in this function and only leave
|
||||||
|
* via the out_unlock label.
|
||||||
*/
|
*/
|
||||||
static void event_srv_chk_w(struct conn_stream *cs)
|
static void event_srv_chk_w(struct conn_stream *cs)
|
||||||
{
|
{
|
||||||
@ -716,10 +719,8 @@ static void event_srv_chk_w(struct conn_stream *cs)
|
|||||||
if (unlikely(check->result == CHK_RES_FAILED))
|
if (unlikely(check->result == CHK_RES_FAILED))
|
||||||
goto out_wakeup;
|
goto out_wakeup;
|
||||||
|
|
||||||
if (conn->flags & CO_FL_HANDSHAKE) {
|
if (conn->flags & CO_FL_HANDSHAKE)
|
||||||
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
goto out_unlock;
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (retrieve_errno_from_socket(conn)) {
|
if (retrieve_errno_from_socket(conn)) {
|
||||||
chk_report_conn_err(check, errno, 0);
|
chk_report_conn_err(check, errno, 0);
|
||||||
@ -741,10 +742,8 @@ static void event_srv_chk_w(struct conn_stream *cs)
|
|||||||
goto out_wakeup;
|
goto out_wakeup;
|
||||||
|
|
||||||
/* wake() will take care of calling tcpcheck_main() */
|
/* wake() will take care of calling tcpcheck_main() */
|
||||||
if (check->type == PR_O2_TCPCHK_CHK) {
|
if (check->type == PR_O2_TCPCHK_CHK)
|
||||||
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
goto out_unlock;
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (check->bo->o) {
|
if (check->bo->o) {
|
||||||
conn->mux->snd_buf(cs, check->bo, 0);
|
conn->mux->snd_buf(cs, check->bo, 0);
|
||||||
@ -753,10 +752,8 @@ static void event_srv_chk_w(struct conn_stream *cs)
|
|||||||
__cs_stop_both(cs);
|
__cs_stop_both(cs);
|
||||||
goto out_wakeup;
|
goto out_wakeup;
|
||||||
}
|
}
|
||||||
if (check->bo->o) {
|
if (check->bo->o)
|
||||||
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
goto out_unlock;
|
||||||
return;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/* full request sent, we allow up to <timeout.check> if nonzero for a response */
|
/* full request sent, we allow up to <timeout.check> if nonzero for a response */
|
||||||
@ -769,8 +766,9 @@ static void event_srv_chk_w(struct conn_stream *cs)
|
|||||||
out_wakeup:
|
out_wakeup:
|
||||||
task_wakeup(t, TASK_WOKEN_IO);
|
task_wakeup(t, TASK_WOKEN_IO);
|
||||||
out_nowake:
|
out_nowake:
|
||||||
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
|
||||||
__cs_stop_send(cs); /* nothing more to write */
|
__cs_stop_send(cs); /* nothing more to write */
|
||||||
|
out_unlock:
|
||||||
|
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
@ -786,6 +784,9 @@ static void event_srv_chk_w(struct conn_stream *cs)
|
|||||||
* distinguish between an SSL server and a pure TCP relay). All other cases will
|
* distinguish between an SSL server and a pure TCP relay). All other cases will
|
||||||
* call it with a proper error status like HCHK_STATUS_L7STS, HCHK_STATUS_L6RSP,
|
* call it with a proper error status like HCHK_STATUS_L7STS, HCHK_STATUS_L6RSP,
|
||||||
* etc.
|
* etc.
|
||||||
|
*
|
||||||
|
* Please do NOT place any return statement in this function and only leave
|
||||||
|
* via the out_unlock label.
|
||||||
*/
|
*/
|
||||||
static void event_srv_chk_r(struct conn_stream *cs)
|
static void event_srv_chk_r(struct conn_stream *cs)
|
||||||
{
|
{
|
||||||
@ -802,16 +803,12 @@ static void event_srv_chk_r(struct conn_stream *cs)
|
|||||||
if (unlikely(check->result == CHK_RES_FAILED))
|
if (unlikely(check->result == CHK_RES_FAILED))
|
||||||
goto out_wakeup;
|
goto out_wakeup;
|
||||||
|
|
||||||
if (conn->flags & CO_FL_HANDSHAKE) {
|
if (conn->flags & CO_FL_HANDSHAKE)
|
||||||
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
goto out_unlock;
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
/* wake() will take care of calling tcpcheck_main() */
|
/* wake() will take care of calling tcpcheck_main() */
|
||||||
if (check->type == PR_O2_TCPCHK_CHK) {
|
if (check->type == PR_O2_TCPCHK_CHK)
|
||||||
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
goto out_unlock;
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
/* Warning! Linux returns EAGAIN on SO_ERROR if data are still available
|
/* Warning! Linux returns EAGAIN on SO_ERROR if data are still available
|
||||||
* but the connection was closed on the remote end. Fortunately, recv still
|
* but the connection was closed on the remote end. Fortunately, recv still
|
||||||
@ -1356,12 +1353,13 @@ static void event_srv_chk_r(struct conn_stream *cs)
|
|||||||
conn->flags |= CO_FL_ERROR;
|
conn->flags |= CO_FL_ERROR;
|
||||||
|
|
||||||
task_wakeup(t, TASK_WOKEN_IO);
|
task_wakeup(t, TASK_WOKEN_IO);
|
||||||
|
out_unlock:
|
||||||
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
||||||
return;
|
return;
|
||||||
|
|
||||||
wait_more_data:
|
wait_more_data:
|
||||||
__cs_want_recv(cs);
|
__cs_want_recv(cs);
|
||||||
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
goto out_unlock;
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
@ -1969,6 +1967,9 @@ out:
|
|||||||
/*
|
/*
|
||||||
* manages a server health-check that uses an external process. Returns
|
* manages a server health-check that uses an external process. Returns
|
||||||
* the time the task accepts to wait, or TIME_ETERNITY for infinity.
|
* the time the task accepts to wait, or TIME_ETERNITY for infinity.
|
||||||
|
*
|
||||||
|
* Please do NOT place any return statement in this function and only leave
|
||||||
|
* via the out_unlock label.
|
||||||
*/
|
*/
|
||||||
static struct task *process_chk_proc(struct task *t)
|
static struct task *process_chk_proc(struct task *t)
|
||||||
{
|
{
|
||||||
@ -1981,10 +1982,8 @@ static struct task *process_chk_proc(struct task *t)
|
|||||||
SPIN_LOCK(SERVER_LOCK, &check->server->lock);
|
SPIN_LOCK(SERVER_LOCK, &check->server->lock);
|
||||||
if (!(check->state & CHK_ST_INPROGRESS)) {
|
if (!(check->state & CHK_ST_INPROGRESS)) {
|
||||||
/* no check currently running */
|
/* no check currently running */
|
||||||
if (!expired) { /* woke up too early */
|
if (!expired) /* woke up too early */
|
||||||
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
goto out_unlock;
|
||||||
return t;
|
|
||||||
}
|
|
||||||
|
|
||||||
/* we don't send any health-checks when the proxy is
|
/* we don't send any health-checks when the proxy is
|
||||||
* stopped, the server should not be checked or the check
|
* stopped, the server should not be checked or the check
|
||||||
@ -2091,6 +2090,8 @@ static struct task *process_chk_proc(struct task *t)
|
|||||||
reschedule:
|
reschedule:
|
||||||
while (tick_is_expired(t->expire, now_ms))
|
while (tick_is_expired(t->expire, now_ms))
|
||||||
t->expire = tick_add(t->expire, MS_TO_TICKS(check->inter));
|
t->expire = tick_add(t->expire, MS_TO_TICKS(check->inter));
|
||||||
|
|
||||||
|
out_unlock:
|
||||||
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
||||||
return t;
|
return t;
|
||||||
}
|
}
|
||||||
@ -2098,6 +2099,9 @@ static struct task *process_chk_proc(struct task *t)
|
|||||||
/*
|
/*
|
||||||
* manages a server health-check that uses a connection. Returns
|
* manages a server health-check that uses a connection. Returns
|
||||||
* the time the task accepts to wait, or TIME_ETERNITY for infinity.
|
* the time the task accepts to wait, or TIME_ETERNITY for infinity.
|
||||||
|
*
|
||||||
|
* Please do NOT place any return statement in this function and only leave
|
||||||
|
* via the out_unlock label.
|
||||||
*/
|
*/
|
||||||
static struct task *process_chk_conn(struct task *t)
|
static struct task *process_chk_conn(struct task *t)
|
||||||
{
|
{
|
||||||
@ -2112,10 +2116,8 @@ static struct task *process_chk_conn(struct task *t)
|
|||||||
SPIN_LOCK(SERVER_LOCK, &check->server->lock);
|
SPIN_LOCK(SERVER_LOCK, &check->server->lock);
|
||||||
if (!(check->state & CHK_ST_INPROGRESS)) {
|
if (!(check->state & CHK_ST_INPROGRESS)) {
|
||||||
/* no check currently running */
|
/* no check currently running */
|
||||||
if (!expired) { /* woke up too early */
|
if (!expired) /* woke up too early */
|
||||||
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
goto out_unlock;
|
||||||
return t;
|
|
||||||
}
|
|
||||||
|
|
||||||
/* we don't send any health-checks when the proxy is
|
/* we don't send any health-checks when the proxy is
|
||||||
* stopped, the server should not be checked or the check
|
* stopped, the server should not be checked or the check
|
||||||
@ -2140,8 +2142,8 @@ static struct task *process_chk_conn(struct task *t)
|
|||||||
|
|
||||||
switch (ret) {
|
switch (ret) {
|
||||||
case SF_ERR_UP:
|
case SF_ERR_UP:
|
||||||
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
goto out_unlock;
|
||||||
return t;
|
|
||||||
case SF_ERR_NONE:
|
case SF_ERR_NONE:
|
||||||
/* we allow up to min(inter, timeout.connect) for a connection
|
/* we allow up to min(inter, timeout.connect) for a connection
|
||||||
* to establish but only when timeout.check is set
|
* to establish but only when timeout.check is set
|
||||||
@ -2219,7 +2221,7 @@ static struct task *process_chk_conn(struct task *t)
|
|||||||
chk_report_conn_err(check, 0, expired);
|
chk_report_conn_err(check, 0, expired);
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
goto out_wait; /* timeout not reached, wait again */
|
goto out_unlock; /* timeout not reached, wait again */
|
||||||
}
|
}
|
||||||
|
|
||||||
/* check complete or aborted */
|
/* check complete or aborted */
|
||||||
@ -2265,7 +2267,7 @@ static struct task *process_chk_conn(struct task *t)
|
|||||||
reschedule:
|
reschedule:
|
||||||
while (tick_is_expired(t->expire, now_ms))
|
while (tick_is_expired(t->expire, now_ms))
|
||||||
t->expire = tick_add(t->expire, MS_TO_TICKS(check->inter));
|
t->expire = tick_add(t->expire, MS_TO_TICKS(check->inter));
|
||||||
out_wait:
|
out_unlock:
|
||||||
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
||||||
return t;
|
return t;
|
||||||
}
|
}
|
||||||
@ -2579,6 +2581,9 @@ static char * tcpcheck_get_step_comment(struct check *check, int stepid)
|
|||||||
* both from the connection's wake() callback and from the check scheduling task.
|
* both from the connection's wake() callback and from the check scheduling task.
|
||||||
* It returns 0 on normal cases, or <0 if a close() has happened on an existing
|
* It returns 0 on normal cases, or <0 if a close() has happened on an existing
|
||||||
* connection, presenting the risk of an fd replacement.
|
* connection, presenting the risk of an fd replacement.
|
||||||
|
*
|
||||||
|
* Please do NOT place any return statement in this function and only leave
|
||||||
|
* via the out_unlock label after setting retcode.
|
||||||
*/
|
*/
|
||||||
static int tcpcheck_main(struct check *check)
|
static int tcpcheck_main(struct check *check)
|
||||||
{
|
{
|
||||||
@ -2634,8 +2639,7 @@ static int tcpcheck_main(struct check *check)
|
|||||||
if (s->proxy->timeout.check)
|
if (s->proxy->timeout.check)
|
||||||
t->expire = tick_first(t->expire, t_con);
|
t->expire = tick_first(t->expire, t_con);
|
||||||
}
|
}
|
||||||
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
goto out_unlock;
|
||||||
return retcode;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/* special case: option tcp-check with no rule, a connect is enough */
|
/* special case: option tcp-check with no rule, a connect is enough */
|
||||||
@ -2730,8 +2734,7 @@ static int tcpcheck_main(struct check *check)
|
|||||||
chunk_appendf(&trash, " comment: '%s'", comment);
|
chunk_appendf(&trash, " comment: '%s'", comment);
|
||||||
set_server_check_status(check, HCHK_STATUS_SOCKERR, trash.str);
|
set_server_check_status(check, HCHK_STATUS_SOCKERR, trash.str);
|
||||||
check->current_step = NULL;
|
check->current_step = NULL;
|
||||||
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
goto out_unlock;
|
||||||
return retcode;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if (check->cs)
|
if (check->cs)
|
||||||
@ -2853,8 +2856,7 @@ static int tcpcheck_main(struct check *check)
|
|||||||
if (s->proxy->timeout.check)
|
if (s->proxy->timeout.check)
|
||||||
t->expire = tick_first(t->expire, t_con);
|
t->expire = tick_first(t->expire, t_con);
|
||||||
}
|
}
|
||||||
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
goto out_unlock;
|
||||||
return retcode;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
} /* end 'connect' */
|
} /* end 'connect' */
|
||||||
@ -3059,8 +3061,7 @@ static int tcpcheck_main(struct check *check)
|
|||||||
if (&check->current_step->list != head &&
|
if (&check->current_step->list != head &&
|
||||||
check->current_step->action == TCPCHK_ACT_EXPECT)
|
check->current_step->action == TCPCHK_ACT_EXPECT)
|
||||||
__cs_want_recv(cs);
|
__cs_want_recv(cs);
|
||||||
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
goto out_unlock;
|
||||||
return retcode;
|
|
||||||
|
|
||||||
out_end_tcpcheck:
|
out_end_tcpcheck:
|
||||||
/* collect possible new errors */
|
/* collect possible new errors */
|
||||||
@ -3074,6 +3075,8 @@ static int tcpcheck_main(struct check *check)
|
|||||||
conn->flags |= CO_FL_ERROR;
|
conn->flags |= CO_FL_ERROR;
|
||||||
|
|
||||||
__cs_stop_both(cs);
|
__cs_stop_both(cs);
|
||||||
|
|
||||||
|
out_unlock:
|
||||||
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
|
||||||
return retcode;
|
return retcode;
|
||||||
}
|
}
|
||||||
|
Loading…
x
Reference in New Issue
Block a user