mir/bind
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-09-01 23:25:38 +00:00
Code Issues Releases Wiki Activity

Fix the data race when shutting down dns_adb

Browse Source 
When dns_adb is shutting down, first the adb->shutting_down flag is set
and then task is created that runs shutdown_stage2() that sets the
shutdown flag on names and entries.  However, when dns_adb_createfind()
is called, only the individual shutdown flags are being checked, and the
global adb->shutting_down flag was not checked.  Because of that it was
possible for a different thread to slip in and create new find between
the dns_adb_shutdown() and dns_adb_detach(), but before the
shutdown_stage2() task is complete.  This was detected by
ThreadSanitizer as data race because the zonetable might have been
already detached by dns_view shutdown process and simultaneously
accessed by dns_adb_createfind().

This commit converts the adb->shutting_down to atomic_bool to prevent
the global adb lock when creating the find.
This commit is contained in:
Ondřej Surý
2021-11-15 12:13:22 +01:00
parent 9d94720735
commit 7e002d89b4
1 changed files with 15 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
lib/dns/adb.c
View File

@@ -145,7 +145,7 @@ struct dns_adb {
isc_event_t cevent; isc_event_t cevent;
bool cevent_out; bool cevent_out;
bool shutting_down; atomic_bool shutting_down;
isc_eventlist_t whenshutdown; isc_eventlist_t whenshutdown;
isc_event_t growentries; isc_event_t growentries;
bool growentries_sent; bool growentries_sent;
@@ -1567,7 +1567,7 @@ check_exit(dns_adb_t *adb) {
/* /*
* The caller must be holding the adb lock. * The caller must be holding the adb lock.
*/ */
if (adb->shutting_down) { if (atomic_load(&adb->shutting_down)) {
/* /*
* If there aren't any external references either, we're * If there aren't any external references either, we're
* done. Send the control event to initiate shutdown. * done. Send the control event to initiate shutdown.
@@ -2542,7 +2542,7 @@ dns_adb_create(isc_mem_t *mem, dns_view_t *view, isc_timermgr_t *timermgr,
ISC_EVENT_INIT(&adb->cevent, sizeof(adb->cevent), 0, NULL, 0, NULL, ISC_EVENT_INIT(&adb->cevent, sizeof(adb->cevent), 0, NULL, 0, NULL,
NULL, NULL, NULL, NULL); NULL, NULL, NULL, NULL);
adb->cevent_out = false; adb->cevent_out = false;
adb->shutting_down = false; atomic_init(&adb->shutting_down, false);
ISC_LIST_INIT(adb->whenshutdown); ISC_LIST_INIT(adb->whenshutdown);
adb->nentries = nbuckets[0]; adb->nentries = nbuckets[0];
@@ -2757,7 +2757,7 @@ dns_adb_detach(dns_adb_t **adbx) {
if (need_exit_check) { if (need_exit_check) {
LOCK(&adb->lock); LOCK(&adb->lock);
INSIST(adb->shutting_down); INSIST(atomic_load(&adb->shutting_down));
check_exit(adb); check_exit(adb);
UNLOCK(&adb->lock); UNLOCK(&adb->lock);
} }
@@ -2784,7 +2784,7 @@ dns_adb_whenshutdown(dns_adb_t *adb, isc_task_t *task, isc_event_t **eventp) {
zeroirefcnt = (adb->irefcnt == 0); zeroirefcnt = (adb->irefcnt == 0);
if (adb->shutting_down && zeroirefcnt && if (atomic_load(&adb->shutting_down) && zeroirefcnt &&
isc_refcount_current(&adb->ahrefcnt) == 0) isc_refcount_current(&adb->ahrefcnt) == 0)
{ {
/* /*
@@ -2813,7 +2813,7 @@ shutdown_stage2(isc_task_t *task, isc_event_t *event) {
INSIST(DNS_ADB_VALID(adb)); INSIST(DNS_ADB_VALID(adb));
LOCK(&adb->lock); LOCK(&adb->lock);
INSIST(adb->shutting_down); INSIST(atomic_load(&adb->shutting_down));
adb->cevent_out = false; adb->cevent_out = false;
(void)shutdown_names(adb); (void)shutdown_names(adb);
(void)shutdown_entries(adb); (void)shutdown_entries(adb);
@@ -2833,8 +2833,8 @@ dns_adb_shutdown(dns_adb_t *adb) {
LOCK(&adb->lock); LOCK(&adb->lock);
if (!adb->shutting_down) { if (atomic_compare_exchange_strong(&adb->shutting_down,
adb->shutting_down = true; &(bool){ false }, true)) {
isc_mem_clearwater(adb->mctx); isc_mem_clearwater(adb->mctx);
/* /*
* Isolate shutdown_names and shutdown_entries calls. * Isolate shutdown_names and shutdown_entries calls.
@@ -2885,6 +2885,13 @@ dns_adb_createfind(dns_adb_t *adb, isc_task_t *task, isc_taskaction_t action,
result = ISC_R_UNEXPECTED; result = ISC_R_UNEXPECTED;
POST(result); POST(result);
if (atomic_load(&adb->shutting_down)) {
DP(DEF_LEVEL, "dns_adb_createfind: returning "
"ISC_R_SHUTTINGDOWN");
return (ISC_R_SHUTTINGDOWN);
}
if (now == 0) { if (now == 0) {
isc_stdtime_get(&now); isc_stdtime_get(&now);
} }