MEDIUM: threads/server: Use the server lock to protect health check and cli concurrency

include/proto/checks.h

@@ -37,11 +37,15 @@ extern struct data_cb check_conn_cb;
  */
 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);

src/checks.c

@@ -650,6 +650,7 @@ static void chk_report_conn_err(struct check *check, int errno_bck, int expired)
 		}
 	}
 
+	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 @@ static void chk_report_conn_err(struct check *check, int errno_bck, int expired)
 		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 @@ static void event_srv_chk_r(struct connection *conn)
 	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 @@ static void event_srv_chk_r(struct connection *conn)
 		    !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 @@ static void event_srv_chk_r(struct connection *conn)
 				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 @@ static void event_srv_chk_r(struct connection *conn)
 			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 @@ static void event_srv_chk_r(struct connection *conn)
 			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 @@ static void event_srv_chk_r(struct connection *conn)
 			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 @@ static void event_srv_chk_r(struct connection *conn)
 		break;
 	} /* switch */
 
+	SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
  out_wakeup:
 	/* collect possible new errors */
 	if (conn->flags & CO_FL_ERROR)

src/hlua_fcn.c

@@ -626,7 +626,9 @@ int hlua_server_shut_sess(lua_State *L)
 	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 @@ int hlua_server_set_drain(lua_State *L)
 	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 @@ int hlua_server_set_maint(lua_State *L)
 	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 @@ int hlua_server_set_ready(lua_State *L)
 	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 @@ int hlua_server_check_enable(lua_State *L)
 	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 @@ int hlua_server_check_disable(lua_State *L)
 	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 @@ int hlua_server_check_force_up(lua_State *L)
 
 	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 @@ int hlua_server_check_force_nolb(lua_State *L)
 
 	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 @@ int hlua_server_check_force_down(lua_State *L)
 	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 @@ int hlua_server_agent_enable(lua_State *L)
 	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 @@ int hlua_server_agent_disable(lua_State *L)
 	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 @@ int hlua_server_agent_force_up(lua_State *L)
 	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 @@ int hlua_server_agent_force_down(lua_State *L)
 	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;
 }
 

src/server.c

@@ -880,8 +880,11 @@ void srv_set_stopped(struct server *s, const char *reason, struct check *check)
 	}
 
 	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 @@ void srv_set_running(struct server *s, const char *reason, struct check *check)
 		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 @@ void srv_set_stopping(struct server *s, const char *reason, struct check *check)
 	}
 
 	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 @@ void srv_set_admin_flag(struct server *s, enum srv_admin mode, const char *cause
 	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 @@ void srv_clr_admin_flag(struct server *s, enum srv_admin mode)
 	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 @@ static void srv_propagate_admin_state(struct server *srv)
 		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 @@ static void srv_update_state(struct server *srv, int version, char **params)
 			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 @@ static void srv_update_state(struct server *srv, int version, char **params)
 
 			if (port_str)
 				srv->svc_port = port;
+			SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
 
 			break;
 		default:
@@ -4013,6 +4032,8 @@ static int cli_parse_set_server(char **args, struct appctx *appctx, void *privat
 	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 @@ static int cli_parse_set_server(char **args, struct appctx *appctx, void *privat
 			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 @@ static int cli_parse_set_server(char **args, struct appctx *appctx, void *privat
 			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 @@ static int cli_parse_set_server(char **args, struct appctx *appctx, void *privat
 		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;
 }
 

src/stats.c

@@ -2764,6 +2764,7 @@ static int stats_process_http_post(struct stream_interface *si)
 					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 @@ static int stats_process_http_post(struct stream_interface *si)
 						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++;