Commit 5d5cf12a authored by Ondřej Surý's avatar Ondřej Surý

Merge branch '1148-deadlock-hangs-named-v9_11-v9_14-master' into 'master'

Resolve "deadlock hangs named"

Closes #1148

See merge request isc-projects/bind9!2236
parents f03aaaa6 4db3189d
......@@ -17,6 +17,7 @@
#include <isc/mem.h>
#include <isc/mutex.h>
#include <isc/portset.h>
#include <isc/refcount.h>
#include <isc/safe.h>
#include <isc/sockaddr.h>
#include <isc/socket.h>
......@@ -92,8 +93,9 @@ struct dns_client {
unsigned int find_timeout;
unsigned int find_udpretries;
isc_refcount_t references;
/* Locked */
unsigned int references;
dns_viewlist_t viewlist;
ISC_LIST(struct resctx) resctxs;
ISC_LIST(struct reqctx) reqctxs;
......@@ -498,12 +500,13 @@ dns_client_createx(isc_mem_t *mctx, isc_appctx_t *actx,
client->task = NULL;
result = isc_task_create(client->taskmgr, 0, &client->task);
if (result != ISC_R_SUCCESS)
goto cleanup;
if (result != ISC_R_SUCCESS) {
goto cleanup_lock;
}
result = dns_dispatchmgr_create(mctx, &dispatchmgr);
if (result != ISC_R_SUCCESS)
goto cleanup;
goto cleanup_task;
client->dispatchmgr = dispatchmgr;
(void)setsourceports(mctx, dispatchmgr);
......@@ -516,8 +519,9 @@ dns_client_createx(isc_mem_t *mctx, isc_appctx_t *actx,
result = getudpdispatch(AF_INET, dispatchmgr, socketmgr,
taskmgr, true,
&dispatchv4, localaddr4);
if (result == ISC_R_SUCCESS)
if (result == ISC_R_SUCCESS) {
client->dispatchv4 = dispatchv4;
}
}
client->dispatchv6 = NULL;
......@@ -525,22 +529,27 @@ dns_client_createx(isc_mem_t *mctx, isc_appctx_t *actx,
result = getudpdispatch(AF_INET6, dispatchmgr, socketmgr,
taskmgr, true,
&dispatchv6, localaddr6);
if (result == ISC_R_SUCCESS)
if (result == ISC_R_SUCCESS) {
client->dispatchv6 = dispatchv6;
}
}
/* We need at least one of the dispatchers */
if (dispatchv4 == NULL && dispatchv6 == NULL) {
INSIST(result != ISC_R_SUCCESS);
goto cleanup;
goto cleanup_dispatchmgr;
}
isc_refcount_init(&client->references, 1);
/* Create the default view for class IN */
result = createview(mctx, dns_rdataclass_in, options, taskmgr,
RESOLVER_NTASKS, socketmgr, timermgr,
dispatchmgr, dispatchv4, dispatchv6, &view);
if (result != ISC_R_SUCCESS)
goto cleanup;
if (result != ISC_R_SUCCESS) {
goto cleanup_references;
}
ISC_LIST_INIT(client->viewlist);
ISC_LIST_APPEND(client->viewlist, view, link);
......@@ -560,32 +569,38 @@ dns_client_createx(isc_mem_t *mctx, isc_appctx_t *actx,
client->find_udpretries = DEF_FIND_UDPRETRIES;
client->attributes = 0;
client->references = 1;
client->magic = DNS_CLIENT_MAGIC;
*clientp = client;
return (ISC_R_SUCCESS);
cleanup:
if (dispatchv4 != NULL)
cleanup_references:
isc_refcount_decrement(&client->references);
isc_refcount_destroy(&client->references);
cleanup_dispatchmgr:
if (dispatchv4 != NULL) {
dns_dispatch_detach(&dispatchv4);
if (dispatchv6 != NULL)
}
if (dispatchv6 != NULL) {
dns_dispatch_detach(&dispatchv6);
if (dispatchmgr != NULL)
dns_dispatchmgr_destroy(&dispatchmgr);
if (client->task != NULL)
isc_task_detach(&client->task);
}
dns_dispatchmgr_destroy(&dispatchmgr);
cleanup_task:
isc_task_detach(&client->task);
cleanup_lock:
isc_mutex_destroy(&client->lock);
isc_mem_put(mctx, client, sizeof(*client));
return (result);
}
static void
destroyclient(dns_client_t **clientp) {
dns_client_t *client = *clientp;
destroyclient(dns_client_t *client) {
dns_view_t *view;
isc_refcount_destroy(&client->references);
while ((view = ISC_LIST_HEAD(client->viewlist)) != NULL) {
ISC_LIST_UNLINK(client->viewlist, view, link);
dns_view_detach(&view);
......@@ -617,32 +632,20 @@ destroyclient(dns_client_t **clientp) {
client->magic = 0;
isc_mem_putanddetach(&client->mctx, client, sizeof(*client));
*clientp = NULL;
}
void
dns_client_destroy(dns_client_t **clientp) {
dns_client_t *client;
bool destroyok = false;
REQUIRE(clientp != NULL);
client = *clientp;
*clientp = NULL;
REQUIRE(DNS_CLIENT_VALID(client));
LOCK(&client->lock);
client->references--;
if (client->references == 0 && ISC_LIST_EMPTY(client->resctxs) &&
ISC_LIST_EMPTY(client->reqctxs) &&
ISC_LIST_EMPTY(client->updatectxs)) {
destroyok = true;
if (isc_refcount_decrement(&client->references) == 1) {
destroyclient(client);
}
UNLOCK(&client->lock);
if (destroyok)
destroyclient(&client);
*clientp = NULL;
}
isc_result_t
......@@ -1428,6 +1431,7 @@ dns_client_startresolve(dns_client_t *client, const dns_name_t *name,
rctx->event = event;
rctx->magic = RCTX_MAGIC;
isc_refcount_increment(&client->references);
LOCK(&client->lock);
ISC_LIST_APPEND(client->resctxs, rctx, link);
......@@ -1498,10 +1502,10 @@ dns_client_destroyrestrans(dns_clientrestrans_t **transp) {
resctx_t *rctx;
isc_mem_t *mctx;
dns_client_t *client;
bool need_destroyclient = false;
REQUIRE(transp != NULL);
rctx = (resctx_t *)*transp;
*transp = NULL;
REQUIRE(RCTX_VALID(rctx));
REQUIRE(rctx->fetch == NULL);
REQUIRE(rctx->event == NULL);
......@@ -1523,11 +1527,6 @@ dns_client_destroyrestrans(dns_clientrestrans_t **transp) {
INSIST(ISC_LINK_LINKED(rctx, link));
ISC_LIST_UNLINK(client->resctxs, rctx, link);
if (client->references == 0 && ISC_LIST_EMPTY(client->resctxs) &&
ISC_LIST_EMPTY(client->reqctxs) &&
ISC_LIST_EMPTY(client->updatectxs))
need_destroyclient = true;
UNLOCK(&client->lock);
INSIST(ISC_LIST_EMPTY(rctx->namelist));
......@@ -1537,10 +1536,8 @@ dns_client_destroyrestrans(dns_clientrestrans_t **transp) {
isc_mem_put(mctx, rctx, sizeof(*rctx));
if (need_destroyclient)
destroyclient(&client);
dns_client_destroy(&client);
*transp = NULL;
}
isc_result_t
......@@ -1804,6 +1801,7 @@ dns_client_startrequest(dns_client_t *client, dns_message_t *qmessage,
LOCK(&client->lock);
ISC_LIST_APPEND(client->reqctxs, ctx, link);
isc_refcount_increment(&client->references);
UNLOCK(&client->lock);
ctx->request = NULL;
......@@ -1818,6 +1816,8 @@ dns_client_startrequest(dns_client_t *client, dns_message_t *qmessage,
return (ISC_R_SUCCESS);
}
isc_refcount_decrement(&client->references);
cleanup:
if (ctx != NULL) {
LOCK(&client->lock);
......@@ -1858,10 +1858,10 @@ dns_client_destroyreqtrans(dns_clientreqtrans_t **transp) {
reqctx_t *ctx;
isc_mem_t *mctx;
dns_client_t *client;
bool need_destroyclient = false;
REQUIRE(transp != NULL);
ctx = (reqctx_t *)*transp;
*transp = NULL;
REQUIRE(REQCTX_VALID(ctx));
client = ctx->client;
REQUIRE(DNS_CLIENT_VALID(client));
......@@ -1876,12 +1876,6 @@ dns_client_destroyreqtrans(dns_clientreqtrans_t **transp) {
INSIST(ISC_LINK_LINKED(ctx, link));
ISC_LIST_UNLINK(client->reqctxs, ctx, link);
if (client->references == 0 && ISC_LIST_EMPTY(client->resctxs) &&
ISC_LIST_EMPTY(client->reqctxs) &&
ISC_LIST_EMPTY(client->updatectxs)) {
need_destroyclient = true;
}
UNLOCK(&client->lock);
isc_mutex_destroy(&ctx->lock);
......@@ -1889,10 +1883,7 @@ dns_client_destroyreqtrans(dns_clientreqtrans_t **transp) {
isc_mem_put(mctx, ctx, sizeof(*ctx));
if (need_destroyclient)
destroyclient(&client);
*transp = NULL;
dns_client_destroy(&client);
}
/*%
......@@ -2962,6 +2953,7 @@ dns_client_startupdate(dns_client_t *client, dns_rdataclass_t rdclass,
LOCK(&client->lock);
ISC_LIST_APPEND(client->updatectxs, uctx, link);
isc_refcount_increment(&client->references);
UNLOCK(&client->lock);
*transp = (dns_clientupdatetrans_t *)uctx;
......@@ -2980,6 +2972,8 @@ dns_client_startupdate(dns_client_t *client, dns_rdataclass_t rdclass,
}
if (result == ISC_R_SUCCESS)
return (result);
isc_refcount_decrement(&client->references);
*transp = NULL;
fail:
......@@ -3037,11 +3031,11 @@ dns_client_destroyupdatetrans(dns_clientupdatetrans_t **transp) {
updatectx_t *uctx;
isc_mem_t *mctx;
dns_client_t *client;
bool need_destroyclient = false;
isc_sockaddr_t *sa;
REQUIRE(transp != NULL);
uctx = (updatectx_t *)*transp;
*transp = NULL;
REQUIRE(UCTX_VALID(uctx));
client = uctx->client;
REQUIRE(DNS_CLIENT_VALID(client));
......@@ -3062,11 +3056,6 @@ dns_client_destroyupdatetrans(dns_clientupdatetrans_t **transp) {
INSIST(ISC_LINK_LINKED(uctx, link));
ISC_LIST_UNLINK(client->updatectxs, uctx, link);
if (client->references == 0 && ISC_LIST_EMPTY(client->resctxs) &&
ISC_LIST_EMPTY(client->reqctxs) &&
ISC_LIST_EMPTY(client->updatectxs))
need_destroyclient = true;
UNLOCK(&client->lock);
isc_mutex_destroy(&uctx->lock);
......@@ -3074,10 +3063,7 @@ dns_client_destroyupdatetrans(dns_clientupdatetrans_t **transp) {
isc_mem_put(mctx, uctx, sizeof(*uctx));
if (need_destroyclient)
destroyclient(&client);
*transp = NULL;
dns_client_destroy(&client);
}
isc_mem_t *
......
......@@ -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;
......
......@@ -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, 1);
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,11 @@ 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_decrement(&view->weakrefs);
isc_refcount_destroy(&view->weakrefs);
isc_refcount_decrement(&view->references);
isc_refcount_destroy(&view->references);
if (view->fwdtable != NULL) {
......@@ -337,12 +340,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 +539,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) {
......@@ -546,21 +552,6 @@ destroy(dns_view_t *view) {
isc_mem_putanddetach(&view->mctx, view, sizeof(*view));
}
/*
* Return true iff 'view' may be freed.
* The caller must be holding the view lock.
*/
static bool
all_done(dns_view_t *view) {
if (isc_refcount_current(&view->references) == 0 &&
view->weakrefs == 0 &&
RESSHUTDOWN(view) && ADBSHUTDOWN(view) && REQSHUTDOWN(view))
return (true);
return (false);
}
void
dns_view_attach(dns_view_t *source, dns_view_t **targetp) {
......@@ -582,7 +573,6 @@ view_flushanddetach(dns_view_t **viewp, bool flush) {
view->flush = flush;
}
bool done = false;
if (isc_refcount_decrement(&view->references) == 1) {
dns_zone_t *mkzone = NULL, *rdzone = NULL;
......@@ -620,7 +610,6 @@ view_flushanddetach(dns_view_t **viewp, bool flush) {
if (view->catzs != NULL) {
dns_catz_catzs_detach(&view->catzs);
}
done = all_done(view);
UNLOCK(&view->lock);
/* Need to detach zones outside view lock */
......@@ -631,12 +620,8 @@ view_flushanddetach(dns_view_t **viewp, bool flush) {
if (rdzone != NULL) {
dns_zone_detach(&rdzone);
}
}
*viewp = NULL;
if (done) {
destroy(view);
dns_view_weakdetach(&view);
}
}
......@@ -671,9 +656,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_increment(&source->weakrefs);
*targetp = source;
}
......@@ -681,30 +664,20 @@ 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)
if (isc_refcount_decrement(&view->weakrefs) == 1) {
destroy(view);
}
}
static void
resolver_shutdown(isc_task_t *task, isc_event_t *event) {
dns_view_t *view = event->ev_arg;
bool done;
REQUIRE(event->ev_type == DNS_EVENT_VIEWRESSHUTDOWN);
REQUIRE(DNS_VIEW_VALID(view));
......@@ -715,20 +688,15 @@ resolver_shutdown(isc_task_t *task, isc_event_t *event) {
isc_event_free(&event);
LOCK(&view->lock);
view->attributes |= DNS_VIEWATTR_RESSHUTDOWN;
done = all_done(view);
UNLOCK(&view->lock);
if (done)
destroy(view);
dns_view_weakdetach(&view);
}
static void
adb_shutdown(isc_task_t *task, isc_event_t *event) {
dns_view_t *view = event->ev_arg;
bool done;
REQUIRE(event->ev_type == DNS_EVENT_VIEWADBSHUTDOWN);
REQUIRE(DNS_VIEW_VALID(view));
......@@ -739,20 +707,15 @@ adb_shutdown(isc_task_t *task, isc_event_t *event) {
isc_event_free(&event);
LOCK(&view->lock);
view->attributes |= DNS_VIEWATTR_ADBSHUTDOWN;
done = all_done(view);
UNLOCK(&view->lock);
if (done)
destroy(view);
dns_view_weakdetach(&view);
}
static void
req_shutdown(isc_task_t *task, isc_event_t *event) {
dns_view_t *view = event->ev_arg;
bool done;
REQUIRE(event->ev_type == DNS_EVENT_VIEWREQSHUTDOWN);
REQUIRE(DNS_VIEW_VALID(view));
......@@ -763,14 +726,10 @@ req_shutdown(isc_task_t *task, isc_event_t *event) {
isc_event_free(&event);
LOCK(&view->lock);
view->attributes |= DNS_VIEWATTR_REQSHUTDOWN;
done = all_done(view);
UNLOCK(&view->lock);
if (done)
destroy(view);
dns_view_weakdetach(&view);
}
isc_result_t
......@@ -819,6 +778,7 @@ dns_view_createresolver(dns_view_t *view,
event = &view->resevent;
dns_resolver_whenshutdown(view->resolver, view->task, &event);
view->attributes &= ~DNS_VIEWATTR_RESSHUTDOWN;
isc_refcount_increment(&view->weakrefs);
result = isc_mem_create(0, 0, &mctx);
if (result != ISC_R_SUCCESS) {
......@@ -836,6 +796,7 @@ dns_view_createresolver(dns_view_t *view,
event = &view->adbevent;
dns_adb_whenshutdown(view->adb, view->task, &event);
view->attributes &= ~DNS_VIEWATTR_ADBSHUTDOWN;
isc_refcount_increment(&view->weakrefs);
result = dns_requestmgr_create(view->mctx, timermgr, socketmgr,
dns_resolver_taskmgr(view->resolver),
......@@ -850,6 +811,7 @@ dns_view_createresolver(dns_view_t *view,
event = &view->reqevent;
dns_requestmgr_whenshutdown(view->requestmgr, view->task, &event);
view->attributes &= ~DNS_VIEWATTR_REQSHUTDOWN;
isc_refcount_increment(&view->weakrefs);
return (ISC_R_SUCCESS);
}
......
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