MEDIUM: threads/server: Use the server lock to protect health check and cli concurrency
diff --git a/include/proto/checks.h b/include/proto/checks.h
index 98dca25..b0b8c7d 100644
--- a/include/proto/checks.h
+++ b/include/proto/checks.h
@@ -37,11 +37,15 @@
*/
static inline void health_adjust(struct server *s, short status)
{
+ SPIN_LOCK(SERVER_LOCK, &s->lock);
/* return now if observing nor health check is not enabled */
- if (!s->observe || !s->check.task)
+ if (!s->observe || !s->check.task) {
+ SPIN_UNLOCK(SERVER_LOCK, &s->lock);
return;
+ }
- return __health_adjust(s, status);
+ __health_adjust(s, status);
+ SPIN_UNLOCK(SERVER_LOCK, &s->lock);
}
const char *init_check(struct check *check, int type);
diff --git a/src/checks.c b/src/checks.c
index d07e508..8411fef 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -650,6 +650,7 @@
}
}
+ SPIN_LOCK(SERVER_LOCK, &check->server->lock);
if (check->state & CHK_ST_PORT_MISS) {
/* NOTE: this is reported after <fall> tries */
chunk_printf(chk, "No port available for the TCP connection");
@@ -692,6 +693,7 @@
else /* HTTP, SMTP, ... */
set_server_check_status(check, HCHK_STATUS_L7TOUT, err_msg);
}
+ SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
return;
}
@@ -784,15 +786,21 @@
int done;
unsigned short msglen;
+ SPIN_LOCK(SERVER_LOCK, &check->server->lock);
+
if (unlikely(check->result == CHK_RES_FAILED))
goto out_wakeup;
- if (conn->flags & CO_FL_HANDSHAKE)
+ if (conn->flags & CO_FL_HANDSHAKE) {
+ SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
return;
+ }
/* 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);
return;
+ }
/* Warning! Linux returns EAGAIN on SO_ERROR if data are still available
* but the connection was closed on the remote end. Fortunately, recv still
@@ -896,7 +904,6 @@
!isdigit((unsigned char) *(check->bi->data + 2))) {
cut_crlf(check->bi->data);
set_server_check_status(check, HCHK_STATUS_L7RSP, check->bi->data);
-
goto out_wakeup;
}
@@ -1163,6 +1170,7 @@
else {
if (!done)
goto wait_more_data;
+
/* it seems we have a OK packet but without a valid length,
* it must be a protocol error
*/
@@ -1224,6 +1232,7 @@
else {
if (!done)
goto wait_more_data;
+
/* it seems we have a Handshake Initialization packet but without a valid length,
* it must be a protocol error
*/
@@ -1260,7 +1269,6 @@
if ((msglen > 2) ||
(memcmp(check->bi->data + 2 + msglen, "\x02\x01\x01\x61", 4) != 0)) {
set_server_check_status(check, HCHK_STATUS_L7RSP, "Not LDAPv3 protocol");
-
goto out_wakeup;
}
@@ -1273,7 +1281,6 @@
if ((msglen > 4) ||
(memcmp(check->bi->data + 7 + msglen, "\x0a\x01", 2) != 0)) {
set_server_check_status(check, HCHK_STATUS_L7RSP, "Not LDAPv3 protocol");
-
goto out_wakeup;
}
@@ -1314,6 +1321,7 @@
break;
} /* switch */
+ SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
out_wakeup:
/* collect possible new errors */
if (conn->flags & CO_FL_ERROR)
diff --git a/src/hlua_fcn.c b/src/hlua_fcn.c
index a4adb21..60becb0 100644
--- a/src/hlua_fcn.c
+++ b/src/hlua_fcn.c
@@ -626,7 +626,9 @@
struct server *srv;
srv = hlua_check_server(L, 1);
+ SPIN_LOCK(SERVER_LOCK, &srv->lock);
srv_shutdown_streams(srv, SF_ERR_KILLED);
+ SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
return 0;
}
@@ -635,7 +637,9 @@
struct server *srv;
srv = hlua_check_server(L, 1);
+ SPIN_LOCK(SERVER_LOCK, &srv->lock);
srv_adm_set_drain(srv);
+ SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
return 0;
}
@@ -644,7 +648,9 @@
struct server *srv;
srv = hlua_check_server(L, 1);
+ SPIN_LOCK(SERVER_LOCK, &srv->lock);
srv_adm_set_maint(srv);
+ SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
return 0;
}
@@ -653,7 +659,9 @@
struct server *srv;
srv = hlua_check_server(L, 1);
+ SPIN_LOCK(SERVER_LOCK, &srv->lock);
srv_adm_set_ready(srv);
+ SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
return 0;
}
@@ -662,9 +670,11 @@
struct server *sv;
sv = hlua_check_server(L, 1);
+ SPIN_LOCK(SERVER_LOCK, &sv->lock);
if (sv->check.state & CHK_ST_CONFIGURED) {
sv->check.state |= CHK_ST_ENABLED;
}
+ SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
return 0;
}
@@ -673,9 +683,11 @@
struct server *sv;
sv = hlua_check_server(L, 1);
+ SPIN_LOCK(SERVER_LOCK, &sv->lock);
if (sv->check.state & CHK_ST_CONFIGURED) {
sv->check.state &= ~CHK_ST_ENABLED;
}
+ SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
return 0;
}
@@ -685,8 +697,10 @@
sv = hlua_check_server(L, 1);
if (!(sv->track)) {
+ SPIN_LOCK(SERVER_LOCK, &sv->lock);
sv->check.health = sv->check.rise + sv->check.fall - 1;
srv_set_running(sv, "changed from Lua script", NULL);
+ SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
}
return 0;
}
@@ -697,8 +711,10 @@
sv = hlua_check_server(L, 1);
if (!(sv->track)) {
+ SPIN_LOCK(SERVER_LOCK, &sv->lock);
sv->check.health = sv->check.rise + sv->check.fall - 1;
srv_set_stopping(sv, "changed from Lua script", NULL);
+ SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
}
return 0;
}
@@ -708,10 +724,12 @@
struct server *sv;
sv = hlua_check_server(L, 1);
+ SPIN_LOCK(SERVER_LOCK, &sv->lock);
if (!(sv->track)) {
sv->check.health = 0;
srv_set_stopped(sv, "changed from Lua script", NULL);
}
+ SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
return 0;
}
@@ -720,9 +738,11 @@
struct server *sv;
sv = hlua_check_server(L, 1);
+ SPIN_LOCK(SERVER_LOCK, &sv->lock);
if (sv->agent.state & CHK_ST_CONFIGURED) {
sv->agent.state |= CHK_ST_ENABLED;
}
+ SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
return 0;
}
@@ -731,9 +751,11 @@
struct server *sv;
sv = hlua_check_server(L, 1);
+ SPIN_LOCK(SERVER_LOCK, &sv->lock);
if (sv->agent.state & CHK_ST_CONFIGURED) {
sv->agent.state &= ~CHK_ST_ENABLED;
}
+ SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
return 0;
}
@@ -742,10 +764,12 @@
struct server *sv;
sv = hlua_check_server(L, 1);
+ SPIN_LOCK(SERVER_LOCK, &sv->lock);
if (sv->agent.state & CHK_ST_ENABLED) {
sv->agent.health = sv->agent.rise + sv->agent.fall - 1;
srv_set_running(sv, "changed from Lua script", NULL);
}
+ SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
return 0;
}
@@ -754,10 +778,12 @@
struct server *sv;
sv = hlua_check_server(L, 1);
+ SPIN_LOCK(SERVER_LOCK, &sv->lock);
if (sv->agent.state & CHK_ST_ENABLED) {
sv->agent.health = 0;
srv_set_stopped(sv, "changed from Lua script", NULL);
}
+ SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
return 0;
}
diff --git a/src/server.c b/src/server.c
index f0b912f..2d0e3b4 100644
--- a/src/server.c
+++ b/src/server.c
@@ -880,8 +880,11 @@
}
srv_register_update(s);
- for (srv = s->trackers; srv; srv = srv->tracknext)
+ for (srv = s->trackers; srv; srv = srv->tracknext) {
+ SPIN_LOCK(SERVER_LOCK, &srv->lock);
srv_set_stopped(srv, NULL, NULL);
+ SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
+ }
}
/* Marks server <s> up regardless of its checks' statuses and provided it isn't
@@ -919,8 +922,11 @@
s->next_state = SRV_ST_RUNNING;
srv_register_update(s);
- for (srv = s->trackers; srv; srv = srv->tracknext)
+ for (srv = s->trackers; srv; srv = srv->tracknext) {
+ SPIN_LOCK(SERVER_LOCK, &srv->lock);
srv_set_running(srv, NULL, NULL);
+ SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
+ }
}
/* Marks server <s> stopping regardless of its checks' statuses and provided it
@@ -957,8 +963,11 @@
}
srv_register_update(s);
- for (srv = s->trackers; srv; srv = srv->tracknext)
+ for (srv = s->trackers; srv; srv = srv->tracknext) {
+ SPIN_LOCK(SERVER_LOCK, &srv->lock);
srv_set_stopping(srv, NULL, NULL);
+ SPIN_LOCK(SERVER_LOCK, &srv->lock);
+ }
}
/* Enables admin flag <mode> (among SRV_ADMF_*) on server <s>. This is used to
@@ -997,8 +1006,11 @@
else if (mode & SRV_ADMF_DRAIN)
mode = SRV_ADMF_IDRAIN;
- for (srv = s->trackers; srv; srv = srv->tracknext)
+ for (srv = s->trackers; srv; srv = srv->tracknext) {
+ SPIN_LOCK(SERVER_LOCK, &srv->lock);
srv_set_admin_flag(srv, mode, cause);
+ SPIN_LOCK(SERVER_LOCK, &srv->lock);
+ }
}
/* Disables admin flag <mode> (among SRV_ADMF_*) on server <s>. This is used to
@@ -1032,8 +1044,11 @@
else if (mode & SRV_ADMF_DRAIN)
mode = SRV_ADMF_IDRAIN;
- for (srv = s->trackers; srv; srv = srv->tracknext)
+ for (srv = s->trackers; srv; srv = srv->tracknext) {
+ SPIN_LOCK(SERVER_LOCK, &srv->lock);
srv_clr_admin_flag(srv, mode);
+ SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
+ }
}
/* principle: propagate maint and drain to tracking servers. This is useful
@@ -1047,11 +1062,13 @@
return;
for (srv2 = srv->trackers; srv2; srv2 = srv2->tracknext) {
+ SPIN_LOCK(SERVER_LOCK, &srv2->lock);
if (srv->next_admin & (SRV_ADMF_MAINT | SRV_ADMF_CMAINT))
srv_set_admin_flag(srv2, SRV_ADMF_IMAINT, NULL);
if (srv->next_admin & SRV_ADMF_DRAIN)
srv_set_admin_flag(srv2, SRV_ADMF_IDRAIN, NULL);
+ SPIN_UNLOCK(SERVER_LOCK, &srv2->lock);
}
}
@@ -2772,6 +2789,7 @@
if (msg->len)
goto out;
+ SPIN_LOCK(SERVER_LOCK, &srv->lock);
/* recover operational state and apply it to this server
* and all servers tracking this one */
switch (srv_op_state) {
@@ -2901,6 +2919,7 @@
if (port_str)
srv->svc_port = port;
+ SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
break;
default:
@@ -4013,6 +4032,8 @@
if (!sv)
return 1;
+ SPIN_LOCK(SERVER_LOCK, &sv->lock);
+
if (strcmp(args[3], "weight") == 0) {
warning = server_parse_weight_change_request(sv, args[4]);
if (warning) {
@@ -4126,6 +4147,7 @@
appctx->ctx.cli.severity = LOG_ERR;
appctx->ctx.cli.msg = "can't unset 'port' since MAPPORTS is in use.\n";
appctx->st0 = CLI_ST_PRINT;
+ SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
return 1;
}
sv->check.port = i;
@@ -4140,6 +4162,7 @@
appctx->ctx.cli.severity = LOG_ERR;
appctx->ctx.cli.msg = "set server <b>/<s> addr requires an address and optionally a port.\n";
appctx->st0 = CLI_ST_PRINT;
+ SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
return 1;
}
else {
@@ -4174,6 +4197,7 @@
appctx->ctx.cli.msg = "'set server <srv>' only supports 'agent', 'health', 'state', 'weight', 'addr', 'fqdn' and 'check-port'.\n";
appctx->st0 = CLI_ST_PRINT;
}
+ SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
return 1;
}
diff --git a/src/stats.c b/src/stats.c
index e026f36..f688319 100644
--- a/src/stats.c
+++ b/src/stats.c
@@ -2764,6 +2764,7 @@
reprocess = 1;
}
else if ((sv = findserver(px, value)) != NULL) {
+ SPIN_LOCK(SERVER_LOCK, &sv->lock);
switch (action) {
case ST_ADM_ACTION_DISABLE:
if (!(sv->cur_admin & SRV_ADMF_FMAINT)) {
@@ -2880,17 +2881,16 @@
if (px->state != PR_STSTOPPED) {
struct stream *sess, *sess_bck;
- SPIN_LOCK(SERVER_LOCK, &sv->lock);
list_for_each_entry_safe(sess, sess_bck, &sv->actconns, by_srv)
if (sess->srv_conn == sv)
stream_shutdown(sess, SF_ERR_KILLED);
- SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
altered_servers++;
total_servers++;
}
break;
}
+ SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
} else {
/* the server name is unknown or ambiguous (duplicate names) */
total_servers++;