Commit 151852f4 authored by Ondřej Surý's avatar Ondřej Surý
Browse files

Fix datarace when UDP/TCP connect fails and we are in nmthread

When we were in nmthread, the isc__nm_async_<proto>connect() function
executes in the same thread as the isc__nm_<proto>connect() and on a
failure, it would block indefinitely because the failure branch was
setting sock->active to false before the condition around the wait had a
chance to skip the WAIT().

This also fixes the zero system test being stuck on FreeBSD 11, so we
re-enable the test in the commit.
parent 8e1a05c8
Pipeline #58475 failed with stages
in 31 minutes and 45 seconds
#!/bin/sh
#
# Copyright (C) Internet Systems Consortium, Inc. ("ISC")
#
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, you can obtain one at https://mozilla.org/MPL/2.0/.
#
# See the COPYRIGHT file distributed with this work for additional
# information regarding copyright ownership.
. ../conf.sh
if [ "$(uname -s)" != "Linux" ]; then
echo_i "This test is currently broken on non-Linux platforms"
exit 255
fi
exit 0
......@@ -158,6 +158,7 @@ failed_connect_cb(isc_nmsocket_t *sock, isc__nm_uvreq_t *req,
static isc_result_t
tcp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
isc__networker_t *worker = NULL;
isc_result_t result = ISC_R_DEFAULT;
int r;
REQUIRE(VALID_NMSOCK(sock));
......@@ -182,7 +183,7 @@ tcp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
if (r != 0) {
isc__nm_closesocket(sock->fd);
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_OPENFAIL]);
goto failure;
goto done;
}
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_OPEN]);
......@@ -191,7 +192,7 @@ tcp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
if (r != 0) {
isc__nm_incstats(sock->mgr,
sock->statsindex[STATID_BINDFAIL]);
goto failure;
goto done;
}
}
......@@ -201,20 +202,25 @@ tcp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
if (r != 0) {
isc__nm_incstats(sock->mgr,
sock->statsindex[STATID_CONNECTFAIL]);
goto failure;
goto done;
}
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_CONNECT]);
atomic_store(&sock->connected, true);
return (ISC_R_SUCCESS);
failure:
atomic_store(&sock->active, false);
done:
result = isc__nm_uverr2result(r);
isc__nm_tcp_close(sock);
LOCK(&sock->lock);
sock->result = result;
SIGNAL(&sock->cond);
if (!atomic_load(&sock->active)) {
WAIT(&sock->scond, &sock->lock);
}
INSIST(atomic_load(&sock->active));
UNLOCK(&sock->lock);
return (isc__nm_uverr2result(r));
return (result);
}
void
......@@ -234,22 +240,12 @@ isc__nm_async_tcpconnect(isc__networker_t *worker, isc__netievent_t *ev0) {
REQUIRE(sock->tid == isc_nm_tid());
result = tcp_connect_direct(sock, req);
if (result == ISC_R_SUCCESS) {
atomic_store(&sock->connected, true);
/* The connect cb will be executed in tcp_connect_cb() */
} else {
if (result != ISC_R_SUCCESS) {
atomic_store(&sock->active, false);
isc__nm_tcp_close(sock);
isc__nm_uvreq_put(&req, sock);
}
LOCK(&sock->lock);
sock->result = result;
SIGNAL(&sock->cond);
if (!atomic_load(&sock->active)) {
WAIT(&sock->scond, &sock->lock);
}
INSIST(atomic_load(&sock->active));
UNLOCK(&sock->lock);
/*
* The sock is now attached to the handle.
*/
......@@ -334,7 +330,6 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
sock = isc_mem_get(mgr->mctx, sizeof(*sock));
isc__nmsocket_init(sock, mgr, isc_nm_tcpsocket, local);
atomic_init(&sock->active, false);
sock->extrahandlesize = extrahandlesize;
sock->connect_timeout = timeout;
sock->result = ISC_R_DEFAULT;
......@@ -360,6 +355,7 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
(isc__netievent_t *)ievent);
isc__nm_put_netievent_tcpconnect(mgr, ievent);
} else {
atomic_init(&sock->active, false);
sock->tid = isc_random_uniform(mgr->nworkers);
isc__nm_enqueue_ievent(&mgr->workers[sock->tid],
(isc__netievent_t *)ievent);
......@@ -534,7 +530,7 @@ isc__nm_async_tcplisten(isc__networker_t *worker, isc__netievent_t *ev0) {
if (r < 0) {
isc__nm_closesocket(sock->fd);
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_OPENFAIL]);
goto failure;
goto done;
}
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_OPEN]);
......@@ -547,7 +543,7 @@ isc__nm_async_tcplisten(isc__networker_t *worker, isc__netievent_t *ev0) {
&sock->iface->addr.type.sa, flags);
if (r < 0) {
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_BINDFAIL]);
goto failure;
goto done;
}
#else
if (sock->parent->fd == -1) {
......@@ -556,7 +552,7 @@ isc__nm_async_tcplisten(isc__networker_t *worker, isc__netievent_t *ev0) {
if (r < 0) {
isc__nm_incstats(sock->mgr,
sock->statsindex[STATID_BINDFAIL]);
goto failure;
goto done;
}
sock->parent->uv_handle.tcp.flags = sock->uv_handle.tcp.flags;
sock->parent->fd = sock->fd;
......@@ -578,12 +574,12 @@ isc__nm_async_tcplisten(isc__networker_t *worker, isc__netievent_t *ev0) {
"uv_listen failed: %s",
isc_result_totext(isc__nm_uverr2result(r)));
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_BINDFAIL]);
goto failure;
goto done;
}
atomic_store(&sock->listening, true);
failure:
done:
result = isc__nm_uverr2result(r);
if (result != ISC_R_SUCCESS) {
sock->pquota = NULL;
......
......@@ -197,6 +197,7 @@ failed_connect_cb(isc_nmsocket_t *sock, isc__nm_uvreq_t *req,
static isc_result_t
tcpdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
isc__networker_t *worker = NULL;
isc_result_t result = ISC_R_DEFAULT;
int r;
REQUIRE(VALID_NMSOCK(sock));
......@@ -221,7 +222,7 @@ tcpdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
if (r != 0) {
isc__nm_closesocket(sock->fd);
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_OPENFAIL]);
goto failure;
goto done;
}
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_OPEN]);
......@@ -234,7 +235,7 @@ tcpdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
if (r != 0 && r != UV_EINVAL) {
isc__nm_incstats(sock->mgr,
sock->statsindex[STATID_BINDFAIL]);
goto failure;
goto done;
}
}
......@@ -244,20 +245,25 @@ tcpdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
if (r != 0) {
isc__nm_incstats(sock->mgr,
sock->statsindex[STATID_CONNECTFAIL]);
goto failure;
goto done;
}
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_CONNECT]);
atomic_store(&sock->connected, true);
return (ISC_R_SUCCESS);
failure:
atomic_store(&sock->active, false);
done:
result = isc__nm_uverr2result(r);
isc__nm_tcpdns_close(sock);
LOCK(&sock->lock);
sock->result = result;
SIGNAL(&sock->cond);
if (!atomic_load(&sock->active)) {
WAIT(&sock->scond, &sock->lock);
}
INSIST(atomic_load(&sock->active));
UNLOCK(&sock->lock);
return (isc__nm_uverr2result(r));
return (result);
}
void
......@@ -277,22 +283,12 @@ isc__nm_async_tcpdnsconnect(isc__networker_t *worker, isc__netievent_t *ev0) {
REQUIRE(sock->tid == isc_nm_tid());
result = tcpdns_connect_direct(sock, req);
if (result == ISC_R_SUCCESS) {
atomic_store(&sock->connected, true);
/* The connect cb will be executed in tcpdns_connect_cb() */
} else {
if (result != ISC_R_SUCCESS) {
atomic_store(&sock->active, false);
isc__nm_tcpdns_close(sock);
isc__nm_uvreq_put(&req, sock);
}
LOCK(&sock->lock);
sock->result = result;
SIGNAL(&sock->cond);
if (!atomic_load(&sock->active)) {
WAIT(&sock->scond, &sock->lock);
}
INSIST(atomic_load(&sock->active));
UNLOCK(&sock->lock);
/*
* The sock is now attached to the handle.
*/
......@@ -377,7 +373,6 @@ isc_nm_tcpdnsconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
sock = isc_mem_get(mgr->mctx, sizeof(*sock));
isc__nmsocket_init(sock, mgr, isc_nm_tcpdnssocket, local);
atomic_init(&sock->active, false);
sock->extrahandlesize = extrahandlesize;
sock->connect_timeout = timeout;
sock->result = ISC_R_DEFAULT;
......@@ -403,6 +398,7 @@ isc_nm_tcpdnsconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
(isc__netievent_t *)ievent);
isc__nm_put_netievent_tcpdnsconnect(mgr, ievent);
} else {
atomic_init(&sock->active, false);
sock->tid = isc_random_uniform(mgr->nworkers);
isc__nm_enqueue_ievent(&mgr->workers[sock->tid],
(isc__netievent_t *)ievent);
......@@ -579,7 +575,7 @@ isc__nm_async_tcpdnslisten(isc__networker_t *worker, isc__netievent_t *ev0) {
if (r < 0) {
isc__nm_closesocket(sock->fd);
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_OPENFAIL]);
goto failure;
goto done;
}
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_OPEN]);
......@@ -592,7 +588,7 @@ isc__nm_async_tcpdnslisten(isc__networker_t *worker, isc__netievent_t *ev0) {
&sock->iface->addr.type.sa, flags);
if (r < 0) {
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_BINDFAIL]);
goto failure;
goto done;
}
#else
if (sock->parent->fd == -1) {
......@@ -601,7 +597,7 @@ isc__nm_async_tcpdnslisten(isc__networker_t *worker, isc__netievent_t *ev0) {
if (r < 0) {
isc__nm_incstats(sock->mgr,
sock->statsindex[STATID_BINDFAIL]);
goto failure;
goto done;
}
sock->parent->uv_handle.tcp.flags = sock->uv_handle.tcp.flags;
sock->parent->fd = sock->fd;
......@@ -623,12 +619,12 @@ isc__nm_async_tcpdnslisten(isc__networker_t *worker, isc__netievent_t *ev0) {
"uv_listen failed: %s",
isc_result_totext(isc__nm_uverr2result(r)));
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_BINDFAIL]);
goto failure;
goto done;
}
atomic_store(&sock->listening, true);
failure:
done:
result = isc__nm_uverr2result(r);
if (result != ISC_R_SUCCESS) {
sock->pquota = NULL;
......
......@@ -261,7 +261,7 @@ isc__nm_async_udplisten(isc__networker_t *worker, isc__netievent_t *ev0) {
if (r < 0) {
isc__nm_closesocket(sock->fd);
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_OPENFAIL]);
goto failure;
goto done;
}
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_OPEN]);
......@@ -275,7 +275,7 @@ isc__nm_async_udplisten(isc__networker_t *worker, isc__netievent_t *ev0) {
uv_bind_flags);
if (r < 0) {
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_BINDFAIL]);
goto failure;
goto done;
}
#else
if (sock->parent->fd == -1) {
......@@ -286,7 +286,7 @@ isc__nm_async_udplisten(isc__networker_t *worker, isc__netievent_t *ev0) {
if (r < 0) {
isc__nm_incstats(sock->mgr,
sock->statsindex[STATID_BINDFAIL]);
goto failure;
goto done;
}
sock->parent->uv_handle.udp.flags = sock->uv_handle.udp.flags;
sock->parent->fd = sock->fd;
......@@ -307,12 +307,12 @@ isc__nm_async_udplisten(isc__networker_t *worker, isc__netievent_t *ev0) {
r = uv_udp_recv_start(&sock->uv_handle.udp, udp_alloc_cb, udp_recv_cb);
if (r != 0) {
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_BINDFAIL]);
goto failure;
goto done;
}
atomic_store(&sock->listening, true);
failure:
done:
result = isc__nm_uverr2result(r);
sock->parent->rchildren += 1;
if (sock->parent->result == ISC_R_DEFAULT) {
......@@ -641,6 +641,7 @@ static isc_result_t
udp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
isc__networker_t *worker = NULL;
int uv_bind_flags = UV_UDP_REUSEADDR;
isc_result_t result = ISC_R_DEFAULT;
int r;
REQUIRE(isc__nm_in_netthread());
......@@ -662,7 +663,7 @@ udp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
if (r != 0) {
isc__nm_closesocket(sock->fd);
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_OPENFAIL]);
goto failure;
goto done;
}
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_OPEN]);
......@@ -674,17 +675,8 @@ udp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
uv_bind_flags);
if (r != 0) {
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_BINDFAIL]);
goto failure;
}
r = isc_uv_udp_connect(&sock->uv_handle.udp, &req->peer.type.sa);
if (r != 0) {
isc__nm_incstats(sock->mgr,
sock->statsindex[STATID_CONNECTFAIL]);
goto failure;
goto done;
}
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_CONNECT]);
atomic_store(&sock->connecting, false);
#ifdef ISC_RECV_BUFFER_SIZE
uv_recv_buffer_size(&sock->uv_handle.handle,
......@@ -695,16 +687,30 @@ udp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
&(int){ ISC_SEND_BUFFER_SIZE });
#endif
atomic_store(&sock->connected, true);
r = isc_uv_udp_connect(&sock->uv_handle.udp, &req->peer.type.sa);
if (r != 0) {
isc__nm_incstats(sock->mgr,
sock->statsindex[STATID_CONNECTFAIL]);
goto done;
}
isc__nm_incstats(sock->mgr, sock->statsindex[STATID_CONNECT]);
return (ISC_R_SUCCESS);
atomic_store(&sock->connecting, false);
atomic_store(&sock->connected, true);
failure:
atomic_store(&sock->active, false);
done:
result = isc__nm_uverr2result(r);
isc__nm_udp_close(sock);
LOCK(&sock->lock);
sock->result = result;
SIGNAL(&sock->cond);
if (!atomic_load(&sock->active)) {
WAIT(&sock->scond, &sock->lock);
}
INSIST(atomic_load(&sock->active));
UNLOCK(&sock->lock);
return (isc__nm_uverr2result(r));
return (result);
}
/*
......@@ -730,23 +736,14 @@ isc__nm_async_udpconnect(isc__networker_t *worker, isc__netievent_t *ev0) {
req->handle = isc__nmhandle_get(sock, &req->peer, &sock->iface->addr);
result = udp_connect_direct(sock, req);
if (result != ISC_R_SUCCESS) {
atomic_store(&sock->active, false);
isc__nm_udp_close(sock);
isc__nm_uvreq_put(&req, sock);
}
LOCK(&sock->lock);
sock->result = result;
SIGNAL(&sock->cond);
if (!atomic_load(&sock->active)) {
WAIT(&sock->scond, &sock->lock);
}
INSIST(atomic_load(&sock->active));
UNLOCK(&sock->lock);
/*
* The callback has to be called after the socket has been
* initialized
*/
if (result == ISC_R_SUCCESS) {
} else {
/*
* The callback has to be called after the socket has been
* initialized
*/
isc__nm_connectcb(sock, req, ISC_R_SUCCESS);
}
......@@ -785,7 +782,6 @@ isc_nm_udpconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
sock = isc_mem_get(mgr->mctx, sizeof(isc_nmsocket_t));
isc__nmsocket_init(sock, mgr, isc_nm_udpsocket, local);
atomic_init(&sock->active, false);
sock->connect_cb = cb;
sock->connect_cbarg = cbarg;
sock->read_timeout = timeout;
......@@ -822,6 +818,7 @@ isc_nm_udpconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
(isc__netievent_t *)event);
isc__nm_put_netievent_udpconnect(mgr, event);
} else {
atomic_init(&sock->active, false);
sock->tid = isc_random_uniform(mgr->nworkers);
isc__nm_enqueue_ievent(&mgr->workers[sock->tid],
(isc__netievent_t *)event);
......
......@@ -969,7 +969,6 @@
./bin/tests/system/xferquota/tests.sh SH 2000,2001,2004,2007,2012,2016,2018,2019,2020
./bin/tests/system/zero/ans5/ans.pl PERL 2016,2018,2019,2020
./bin/tests/system/zero/clean.sh SH 2013,2014,2015,2016,2018,2019,2020
./bin/tests/system/zero/prereq.sh SH 2020
./bin/tests/system/zero/setup.sh SH 2013,2014,2016,2018,2019,2020
./bin/tests/system/zero/tests.sh SH 2013,2016,2017,2018,2019,2020
./bin/tests/system/zonechecks/clean.sh SH 2004,2007,2012,2014,2015,2016,2018,2019,2020
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment