Bug 1424834 - Replace nsHostRecord.callbacks with LinkedList<RefPtr<nsResolveHostCallback>> r=mayhemer

* nsResolveHostCallback extends nsISupports (for addref-ing and because nsDNSAsyncRequest implements nsICancelable)
* nsResolveHostCallback extends LinkedListElement<RefPtr<nsResolveHostCallback>> so the list can properly manage references
* nsDNSAsyncRequest and nsDNSSyncRequest properly implement nsISupports and use RefPtr to manage lifetimes


MozReview-Commit-ID: 5NvBcWZzfyN

--HG--
extra : rebase_source : 67adcbd20a29803e5abbb3d16324f9cac6771d28
This commit is contained in:
Valentin Gosu 2017-12-20 01:13:46 +01:00
Родитель 1888227294
Коммит a7fe7a006d
3 изменённых файлов: 77 добавлений и 84 удалений

Просмотреть файл

@ -295,8 +295,6 @@ nsDNSRecord::ReportUnusable(uint16_t aPort)
class nsDNSAsyncRequest final : public nsResolveHostCallback
, public nsICancelable
{
~nsDNSAsyncRequest() = default;
public:
NS_DECL_THREADSAFE_ISUPPORTS
NS_DECL_NSICANCELABLE
@ -331,8 +329,12 @@ public:
uint16_t mFlags;
uint16_t mAF;
nsCString mNetworkInterface;
private:
virtual ~nsDNSAsyncRequest() = default;
};
NS_IMPL_ISUPPORTS(nsDNSAsyncRequest, nsICancelable)
void
nsDNSAsyncRequest::OnResolveHostComplete(nsHostResolver *resolver,
nsHostRecord *hostRecord,
@ -349,10 +351,6 @@ nsDNSAsyncRequest::OnResolveHostComplete(nsHostResolver *resolver,
mListener->OnLookupComplete(this, rec, status);
mListener = nullptr;
// release the reference to ourselves that was added before we were
// handed off to the host resolver.
NS_RELEASE_THIS();
}
bool
@ -380,8 +378,6 @@ nsDNSAsyncRequest::SizeOfIncludingThis(MallocSizeOf mallocSizeOf) const
return n;
}
NS_IMPL_ISUPPORTS(nsDNSAsyncRequest, nsICancelable)
NS_IMETHODIMP
nsDNSAsyncRequest::Cancel(nsresult reason)
{
@ -393,14 +389,15 @@ nsDNSAsyncRequest::Cancel(nsresult reason)
//-----------------------------------------------------------------------------
class nsDNSSyncRequest : public nsResolveHostCallback
class nsDNSSyncRequest
: public nsResolveHostCallback
{
NS_DECL_THREADSAFE_ISUPPORTS
public:
explicit nsDNSSyncRequest(PRMonitor *mon)
: mDone(false)
, mStatus(NS_OK)
, mMonitor(mon) {}
virtual ~nsDNSSyncRequest() = default;
void OnResolveHostComplete(nsHostResolver *, nsHostRecord *, nsresult) override;
bool EqualsAsyncListener(nsIDNSListener *aListener) override;
@ -411,9 +408,13 @@ public:
RefPtr<nsHostRecord> mHostRecord;
private:
virtual ~nsDNSSyncRequest() = default;
PRMonitor *mMonitor;
};
NS_IMPL_ISUPPORTS0(nsDNSSyncRequest)
void
nsDNSSyncRequest::OnResolveHostComplete(nsHostResolver *resolver,
nsHostRecord *hostRecord,
@ -798,7 +799,7 @@ NS_IMETHODIMP
nsDNSService::AsyncResolveExtendedNative(const nsACString &aHostname,
uint32_t flags,
const nsACString &aNetworkInterface,
nsIDNSListener *listener,
nsIDNSListener *aListener,
nsIEventTarget *target_,
const OriginAttributes &aOriginAttributes,
nsICancelable **result)
@ -808,6 +809,7 @@ nsDNSService::AsyncResolveExtendedNative(const nsACString &aHostname,
RefPtr<nsHostResolver> res;
nsCOMPtr<nsIIDNService> idn;
nsCOMPtr<nsIEventTarget> target = target_;
nsCOMPtr<nsIDNSListener> listener = aListener;
bool localDomain = false;
{
MutexAutoLock lock(mLock);
@ -850,21 +852,16 @@ nsDNSService::AsyncResolveExtendedNative(const nsACString &aHostname,
uint16_t af = GetAFForLookup(hostname, flags);
auto *req =
MOZ_ASSERT(listener);
RefPtr<nsDNSAsyncRequest> req =
new nsDNSAsyncRequest(res, hostname, aOriginAttributes, listener, flags, af,
aNetworkInterface);
if (!req)
return NS_ERROR_OUT_OF_MEMORY;
NS_ADDREF(*result = req);
// addref for resolver; will be released when OnResolveHostComplete is called.
NS_ADDREF(req);
rv = res->ResolveHost(req->mHost.get(), req->mOriginAttributes, flags, af,
req->mNetworkInterface.get(), req);
if (NS_FAILED(rv)) {
NS_RELEASE(req);
NS_RELEASE(*result);
}
req.forget(result);
return rv;
}
@ -1055,25 +1052,23 @@ nsDNSService::ResolveInternal(const nsACString &aHostname,
return NS_ERROR_OUT_OF_MEMORY;
PR_EnterMonitor(mon);
nsDNSSyncRequest syncReq(mon);
RefPtr<nsDNSSyncRequest> syncReq = new nsDNSSyncRequest(mon);
uint16_t af = GetAFForLookup(hostname, flags);
rv = res->ResolveHost(hostname.get(), aOriginAttributes, flags, af, "", &syncReq);
rv = res->ResolveHost(hostname.get(), aOriginAttributes, flags, af, "", syncReq);
if (NS_SUCCEEDED(rv)) {
// wait for result
while (!syncReq.mDone)
while (!syncReq->mDone) {
PR_Wait(mon, PR_INTERVAL_NO_TIMEOUT);
}
if (NS_FAILED(syncReq.mStatus))
rv = syncReq.mStatus;
else {
NS_ASSERTION(syncReq.mHostRecord, "no host record");
auto *rec = new nsDNSRecord(syncReq.mHostRecord);
if (!rec)
rv = NS_ERROR_OUT_OF_MEMORY;
else
NS_ADDREF(*result = rec);
if (NS_FAILED(syncReq->mStatus)) {
rv = syncReq->mStatus;
} else {
NS_ASSERTION(syncReq->mHostRecord, "no host record");
RefPtr<nsDNSRecord> rec = new nsDNSRecord(syncReq->mHostRecord);
rec.forget(result);
}
}

Просмотреть файл

@ -189,7 +189,6 @@ nsHostRecord::nsHostRecord(const nsHostKey *key)
memcpy((char *) originSuffix, key->originSuffix,
strlen(key->originSuffix) + 1);
PR_INIT_CLIST(this);
PR_INIT_CLIST(&callbacks);
}
nsresult
@ -230,6 +229,8 @@ nsHostRecord::CopyExpirationTimesAndFlagsFrom(const nsHostRecord *aFromHostRecor
nsHostRecord::~nsHostRecord()
{
mCallbacks.clear();
Telemetry::Accumulate(Telemetry::DNS_BLACKLIST_COUNT, mBlacklistedCount);
delete addr_info;
}
@ -326,17 +327,15 @@ nsHostRecord::HasUsableResult(const mozilla::TimeStamp& now, uint16_t queryFlags
}
static size_t
SizeOfResolveHostCallbackListExcludingHead(const PRCList *head,
SizeOfResolveHostCallbackListExcludingHead(const mozilla::LinkedList<RefPtr<nsResolveHostCallback>>& aCallbacks,
MallocSizeOf mallocSizeOf)
{
size_t n = 0;
PRCList *curr = head->next;
while (curr != head) {
nsResolveHostCallback *callback =
static_cast<nsResolveHostCallback*>(curr);
n += callback->SizeOfIncludingThis(mallocSizeOf);
curr = curr->next;
size_t n = 0; // TODO: should be aCallbacks.sizeOfIncludingThis(mallocSizeOf);
for (const nsResolveHostCallback* t = aCallbacks.getFirst(); t; t = t->getNext()) {
n += t->SizeOfIncludingThis(mallocSizeOf);
}
return n;
}
@ -350,7 +349,7 @@ nsHostRecord::SizeOfIncludingThis(MallocSizeOf mallocSizeOf) const
// nsHostRecord::Create()). So it will be included in the
// |mallocSizeOf(this)| call above.
n += SizeOfResolveHostCallbackListExcludingHead(&callbacks, mallocSizeOf);
n += SizeOfResolveHostCallbackListExcludingHead(mCallbacks, mallocSizeOf);
n += addr_info ? addr_info->SizeOfIncludingThis(mallocSizeOf) : 0;
n += mallocSizeOf(addr.get());
@ -733,7 +732,7 @@ nsHostResolver::ResolveHost(const char *host,
uint16_t flags,
uint16_t af,
const char *netInterface,
nsResolveHostCallback *callback)
nsResolveHostCallback *aCallback)
{
NS_ENSURE_TRUE(host && *host, NS_ERROR_UNEXPECTED);
NS_ENSURE_TRUE(netInterface, NS_ERROR_UNEXPECTED);
@ -746,6 +745,7 @@ nsHostResolver::ResolveHost(const char *host,
if (!net_IsValidHostName(nsDependentCString(host)))
return NS_ERROR_UNKNOWN_HOST;
RefPtr<nsResolveHostCallback> callback(aCallback);
// if result is set inside the lock, then we need to issue the
// callback before returning.
RefPtr<nsHostRecord> result;
@ -931,26 +931,26 @@ nsHostResolver::ResolveHost(const char *host,
LOG_HOST(host, netInterface)));
// Add callback to the list of pending callbacks.
PR_APPEND_LINK(callback, &he->rec->callbacks);
he->rec->mCallbacks.insertBack(callback);
he->rec->flags = flags;
rv = IssueLookup(he->rec);
Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2,
METHOD_NETWORK_FIRST);
if (NS_FAILED(rv)) {
PR_REMOVE_AND_INIT_LINK(callback);
if (NS_FAILED(rv) && callback->isInList()) {
callback->remove();
}
else {
LOG((" DNS lookup for host [%s%s%s] blocking "
"pending 'getaddrinfo' query: callback [%p]",
LOG_HOST(host, netInterface), callback));
LOG_HOST(host, netInterface), callback.get()));
}
}
}
else {
LOG((" Host [%s%s%s] is being resolved. Appending callback "
"[%p].", LOG_HOST(host, netInterface), callback));
"[%p].", LOG_HOST(host, netInterface), callback.get()));
PR_APPEND_LINK(callback, &he->rec->callbacks);
he->rec->mCallbacks.insertBack(callback);
if (he->rec->onQueue) {
Telemetry::Accumulate(Telemetry::DNS_LOOKUP_METHOD2,
METHOD_NETWORK_SHARED);
@ -976,7 +976,11 @@ nsHostResolver::ResolveHost(const char *host,
}
}
}
if (result) {
if (callback->isInList()) {
callback->remove();
}
callback->OnResolveHostComplete(this, result, status);
}
@ -989,10 +993,12 @@ nsHostResolver::DetachCallback(const char *host,
uint16_t flags,
uint16_t af,
const char *netInterface,
nsResolveHostCallback *callback,
nsResolveHostCallback *aCallback,
nsresult status)
{
RefPtr<nsHostRecord> rec;
RefPtr<nsResolveHostCallback> callback(aCallback);
{
MutexAutoLock lock(mLock);
@ -1004,22 +1010,22 @@ nsHostResolver::DetachCallback(const char *host,
if (he) {
// walk list looking for |callback|... we cannot assume
// that it will be there!
PRCList *node = he->rec->callbacks.next;
while (node != &he->rec->callbacks) {
if (static_cast<nsResolveHostCallback *>(node) == callback) {
PR_REMOVE_LINK(callback);
for (nsResolveHostCallback* c: he->rec->mCallbacks) {
if (c == callback) {
rec = he->rec;
c->remove();
break;
}
node = node->next;
}
}
}
// complete callback with the given status code; this would only be done if
// the record was in the process of being resolved.
if (rec)
if (rec) {
callback->OnResolveHostComplete(this, rec, status);
}
}
nsresult
@ -1293,8 +1299,8 @@ nsHostResolver::CompleteLookup(nsHostRecord* rec, nsresult status, AddrInfo* new
{
// get the list of pending callbacks for this lookup, and notify
// them that the lookup is complete.
PRCList cbs;
PR_INIT_CLIST(&cbs);
mozilla::LinkedList<RefPtr<nsResolveHostCallback>> cbs;
{
MutexAutoLock lock(mLock);
@ -1306,7 +1312,7 @@ nsHostResolver::CompleteLookup(nsHostRecord* rec, nsresult status, AddrInfo* new
}
// grab list of callbacks to notify
MoveCList(rec->callbacks, cbs);
cbs = mozilla::Move(rec->mCallbacks);
// update record fields. We might have a rec->addr_info already if a
// previous lookup result expired and we're reresolving it..
@ -1374,15 +1380,8 @@ nsHostResolver::CompleteLookup(nsHostRecord* rec, nsresult status, AddrInfo* new
}
}
if (!PR_CLIST_IS_EMPTY(&cbs)) {
PRCList *node = cbs.next;
while (node != &cbs) {
nsResolveHostCallback *callback =
static_cast<nsResolveHostCallback *>(node);
node = node->next;
callback->OnResolveHostComplete(this, rec, status);
// NOTE: callback must not be dereferenced after this point!!
}
for (nsResolveHostCallback* c = cbs.getFirst(); c; c = c->removeAndGetNext()) {
c->OnResolveHostComplete(this, rec, status);
}
NS_RELEASE(rec);
@ -1410,24 +1409,18 @@ nsHostResolver::CancelAsyncRequest(const char *host,
auto he = static_cast<nsHostDBEnt*>(mDB.Search(&key));
if (he) {
nsHostRecord* recPtr = nullptr;
PRCList *node = he->rec->callbacks.next;
// Remove the first nsDNSAsyncRequest callback which matches the
// supplied listener object
while (node != &he->rec->callbacks) {
nsResolveHostCallback *callback
= static_cast<nsResolveHostCallback *>(node);
if (callback && (callback->EqualsAsyncListener(aListener))) {
// Remove from the list of callbacks
PR_REMOVE_LINK(callback);
for (RefPtr<nsResolveHostCallback> c : he->rec->mCallbacks) {
if (c->EqualsAsyncListener(aListener)) {
c->remove();
recPtr = he->rec;
callback->OnResolveHostComplete(this, recPtr, status);
c->OnResolveHostComplete(this, recPtr, status);
break;
}
node = node->next;
}
// If there are no more callbacks, remove the hash table entry
if (recPtr && PR_CLIST_IS_EMPTY(&recPtr->callbacks)) {
if (recPtr && recPtr->mCallbacks.isEmpty()) {
mDB.Remove((nsHostKey *)recPtr);
// If record is on a Queue, remove it and then deref it
if (recPtr->next != recPtr) {

Просмотреть файл

@ -20,6 +20,7 @@
#include "GetAddrInfo.h"
#include "mozilla/net/DNS.h"
#include "mozilla/net/DashboardTypes.h"
#include "mozilla/LinkedList.h"
#include "mozilla/TimeStamp.h"
#include "mozilla/UniquePtr.h"
@ -130,8 +131,7 @@ public:
private:
friend class nsHostResolver;
PRCList callbacks; /* list of callbacks */
mozilla::LinkedList<RefPtr<nsResolveHostCallback>> mCallbacks;
bool resolving; /* true if this record is being resolved, which means
* that it is either on the pending queue or owned by
@ -163,10 +163,13 @@ private:
};
/**
* ResolveHost callback object. It's PRCList members are used by
* the nsHostResolver and should not be used by anything else.
* This class is used to notify listeners when a ResolveHost operation is
* complete. Classes that derive it must implement threadsafe nsISupports
* to be able to use RefPtr with this class.
*/
class NS_NO_VTABLE nsResolveHostCallback : public PRCList
class nsResolveHostCallback
: public mozilla::LinkedListElement<RefPtr<nsResolveHostCallback>>
, public nsISupports
{
public:
/**
@ -205,6 +208,8 @@ public:
virtual bool EqualsAsyncListener(nsIDNSListener *aListener) = 0;
virtual size_t SizeOfIncludingThis(mozilla::MallocSizeOf) const = 0;
protected:
virtual ~nsResolveHostCallback() = default;
};
/**