mir/bind
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-30 05:57:52 +00:00
Code Issues Releases Wiki Activity

lib/dns/resolver.c: Convert (dns_view_t *)->weakrefs to isc_refcount_t

Browse Source 
There's a deadlock in BIND 9 code where (dns_view_t){ .lock } and
(dns_resolver_t){ .buckets[i].lock } gets locked in different order.  When
view->weakrefs gets converted to a reference counting we can reduce the locking
in dns_view_weakdetach only to cases where it's the last instance of the
dns_view_t object.

(cherry picked from commit a7c9a52c89146490a0e797df2119026a268f294c)
(cherry picked from commit 232140edae6b80f8344db234cda28f8456269a6e)
This commit is contained in:
Ondřej Surý 2019-08-01 11:42:58 +02:00
parent f03aaaa6b5
commit cd9bbe6dea
2 changed files with 25 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
lib/dns/include/dns/view.h
View File

@ -197,9 +197,9 @@ struct dns_view {
/* Locked by themselves. */
isc_refcount_t references;
isc_refcount_t weakrefs;
/* Locked by lock. */
unsigned int weakrefs;
unsigned int attributes;
/* Under owner's locking control. */
ISC_LINK(struct dns_view) link;

46
lib/dns/view.c
View File

@ -139,7 +139,7 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass,
view->frozen = false;
view->task = NULL;
isc_refcount_init(&view->references, 1);
view->weakrefs = 0;
isc_refcount_init(&view->weakrefs, 0);
view->attributes = (DNS_VIEWATTR_RESSHUTDOWN|DNS_VIEWATTR_ADBSHUTDOWN|
DNS_VIEWATTR_REQSHUTDOWN);
view->statickeys = NULL;
@ -149,7 +149,7 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass,
view->matchrecursiveonly = false;
result = dns_tsigkeyring_create(view->mctx, &view->dynamickeys);
if (result != ISC_R_SUCCESS)
goto cleanup_references;
goto cleanup_weakrefs;
view->peers = NULL;
view->order = NULL;
view->delonly = NULL;
@ -304,8 +304,10 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass,
dns_tsigkeyring_detach(&view->dynamickeys);
}
cleanup_references:
INSIST(isc_refcount_decrement(&view->references) > 0);
cleanup_weakrefs:
isc_refcount_destroy(&view->weakrefs);
isc_refcount_decrement(&view->references);
isc_refcount_destroy(&view->references);
if (view->fwdtable != NULL) {
@ -337,12 +339,13 @@ destroy(dns_view_t *view) {
dns_dlzdb_t *dlzdb;
REQUIRE(!ISC_LINK_LINKED(view, link));
REQUIRE(isc_refcount_current(&view->references) == 0);
REQUIRE(view->weakrefs == 0);
REQUIRE(RESSHUTDOWN(view));
REQUIRE(ADBSHUTDOWN(view));
REQUIRE(REQSHUTDOWN(view));
isc_refcount_destroy(&view->references);
isc_refcount_destroy(&view->weakrefs);
if (view->order != NULL)
dns_order_detach(&view->order);
if (view->peers != NULL)
@ -535,6 +538,8 @@ destroy(dns_view_t *view) {
dns_badcache_destroy(&view->failcache);
isc_mutex_destroy(&view->new_zone_lock);
isc_mutex_destroy(&view->lock);
isc_refcount_destroy(&view->references);
isc_refcount_destroy(&view->weakrefs);
isc_mem_free(view->mctx, view->nta_file);
isc_mem_free(view->mctx, view->name);
if (view->hooktable != NULL && view->hooktable_free != NULL) {
@ -554,9 +559,11 @@ static bool
all_done(dns_view_t *view) {
if (isc_refcount_current(&view->references) == 0 &&
view->weakrefs == 0 &&
isc_refcount_current(&view->weakrefs) == 0 &&
RESSHUTDOWN(view) && ADBSHUTDOWN(view) && REQSHUTDOWN(view))
{
return (true);
}
return (false);
}
@ -671,9 +678,7 @@ dns_view_weakattach(dns_view_t *source, dns_view_t **targetp) {
REQUIRE(DNS_VIEW_VALID(source));
REQUIRE(targetp != NULL && *targetp == NULL);
LOCK(&source->lock);
source->weakrefs++;
UNLOCK(&source->lock);
isc_refcount_increment0(&source->weakrefs);
*targetp = source;
}
@ -681,24 +686,21 @@ dns_view_weakattach(dns_view_t *source, dns_view_t **targetp) {
void
dns_view_weakdetach(dns_view_t **viewp) {
dns_view_t *view;
bool done = false;
REQUIRE(viewp != NULL);
view = *viewp;
REQUIRE(DNS_VIEW_VALID(view));
LOCK(&view->lock);
INSIST(view->weakrefs > 0);
view->weakrefs--;
done = all_done(view);
UNLOCK(&view->lock);
*viewp = NULL;
if (done)
destroy(view);
if (isc_refcount_decrement(&view->weakrefs) == 1) {
bool done = false;
LOCK(&view->lock);
done = all_done(view);
UNLOCK(&view->lock);
if (done) {
destroy(view);
}
}
}
static void