commit d7b30508f2271c9fb1daf534302122ea97db7b31
parent 1a065ced7d7cfe816a7451169c75e6de50a16f03
Author: Randell Jesup <rjesup@mozilla.com>
Date: Tue, 7 Oct 2025 17:56:05 +0000
Bug 1978494: Make sure Suspend is called before we initiate async dictionary origin reader r=necko-reviewers,valentin
Differential Revision: https://phabricator.services.mozilla.com/D267529
Diffstat:
6 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/netwerk/cache2/Dictionary.cpp b/netwerk/cache2/Dictionary.cpp
@@ -22,7 +22,6 @@
#include "nsIChannel.h"
#include "nsContentUtils.h"
#include "nsIFile.h"
-#include "nsIHttpChannel.h"
#include "nsIInputStream.h"
#include "nsILoadContext.h"
#include "nsILoadContextInfo.h"
@@ -990,6 +989,7 @@ void DictionaryCache::RemoveAllDictionaries() {
// If it's not in the cache, return nullptr via callback.
void DictionaryCache::GetDictionaryFor(
nsIURI* aURI, ExtContentPolicyType aType, bool& aAsync,
+ nsHttpChannel* aChan, void (*aSuspend)(nsHttpChannel*),
const std::function<nsresult(bool, DictionaryCacheEntry*)>& aCallback) {
aAsync = false;
// Note: IETF 2.2.3 Multiple Matching Directories
@@ -1019,8 +1019,11 @@ void DictionaryCache::GetDictionaryFor(
prepath.get()));
// Wait for the metadata read to complete
RefPtr<DictionaryOriginReader> reader = new DictionaryOriginReader();
- reader->Start(existing.Data(), prepath, aURI, aType, this, aCallback);
+ // Must do this before calling start, which can run the callbacks and call
+ // Resume
aAsync = true;
+ aSuspend(aChan);
+ reader->Start(existing.Data(), prepath, aURI, aType, this, aCallback);
}
return;
}
diff --git a/netwerk/cache2/Dictionary.h b/netwerk/cache2/Dictionary.h
@@ -35,6 +35,7 @@ static const uint32_t METADATA_DICTIONARY_VERSION = 1;
namespace mozilla {
namespace net {
+class nsHttpChannel;
class DictionaryOrigin;
// Outstanding requests that offer this dictionary will hold a reference to it.
@@ -341,6 +342,7 @@ class DictionaryCache final {
// return an entry
void GetDictionaryFor(
nsIURI* aURI, ExtContentPolicyType aType, bool& aAsync,
+ nsHttpChannel* aChan, void (*aSuspend)(nsHttpChannel*),
const std::function<nsresult(bool, DictionaryCacheEntry*)>& aCallback);
size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const {
diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -633,12 +633,16 @@ nsresult nsHttpChannel::PrepareToConnect() {
// This may be async; the dictionary headers may need to fetch an origin
// dictionary cache entry from disk before adding the headers. We can
// continue with channel creation, and just block on this being done later
- bool async;
+ bool async = false;
AUTO_PROFILER_FLOW_MARKER("nsHttpHandler::AddAcceptAndDictionaryHeaders",
NETWORK, Flow::FromPointer(this));
+ // AddAcceptAndDictionaryHeaders must call this->Suspend before kicking
+ // off the async operation that can result in calling the lambda (which
+ // will Resume), to avoid a race condition.
+ bool aAsync;
nsresult rv = gHttpHandler->AddAcceptAndDictionaryHeaders(
mURI, mLoadInfo->GetExternalContentPolicyType(), &mRequestHead, IsHTTPS(),
- async,
+ aAsync, this, nsHttpChannel::StaticSuspend,
[self = RefPtr(this)](bool aNeedsResume, DictionaryCacheEntry* aDict) {
self->mDictDecompress = aDict;
if (aNeedsResume) {
@@ -7143,6 +7147,9 @@ nsHttpChannel::Suspend() {
return NS_FAILED(rvTransaction) ? rvTransaction : rvCache;
}
+// static
+void nsHttpChannel::StaticSuspend(nsHttpChannel* aChan) { aChan->Suspend(); }
+
NS_IMETHODIMP
nsHttpChannel::Resume() {
NS_ENSURE_TRUE(mSuspendCount > 0, NS_ERROR_UNEXPECTED);
diff --git a/netwerk/protocol/http/nsHttpChannel.h b/netwerk/protocol/http/nsHttpChannel.h
@@ -139,6 +139,7 @@ class nsHttpChannel final : public HttpBaseChannel,
const nsACString& reason) override;
NS_IMETHOD Cancel(nsresult status) override;
NS_IMETHOD Suspend() override;
+ static void StaticSuspend(nsHttpChannel* aChan);
NS_IMETHOD Resume() override;
// nsIChannel
NS_IMETHOD
diff --git a/netwerk/protocol/http/nsHttpHandler.cpp b/netwerk/protocol/http/nsHttpHandler.cpp
@@ -654,10 +654,13 @@ nsresult nsHttpHandler::InitConnectionMgr() {
}
// We're using RequestOverride because this can get called when these are
-// set by Fetch from the old request
+// set by Fetch from the old request. We need to pass a function pointer to
+// let GetDictionaryFor suspend the channel before starting the async
+// dictionary load.
nsresult nsHttpHandler::AddAcceptAndDictionaryHeaders(
nsIURI* aURI, ExtContentPolicyType aType, nsHttpRequestHead* aRequest,
- bool aSecure, bool& aAsync,
+ bool aSecure, bool& aAsync, nsHttpChannel* aChan,
+ void (*aSuspend)(nsHttpChannel*),
const std::function<bool(bool, DictionaryCacheEntry*)>& aCallback) {
LOG(("Adding Dictionary headers"));
auto guard = MakeScopeExit([&]() { (aCallback)(false, nullptr); });
@@ -671,7 +674,7 @@ nsresult nsHttpHandler::AddAcceptAndDictionaryHeaders(
// aCallback will now be owned by GetDictionaryFor
guard.release();
mDictionaryCache->GetDictionaryFor(
- aURI, aType, aAsync,
+ aURI, aType, aAsync, aChan, aSuspend,
[self = RefPtr(this), aRequest, aCallback](
bool aNeedsResume, DictionaryCacheEntry* aDict) {
if (!aDict) {
diff --git a/netwerk/protocol/http/nsHttpHandler.h b/netwerk/protocol/http/nsHttpHandler.h
@@ -119,7 +119,8 @@ class nsHttpHandler final : public nsIHttpProtocolHandler,
[[nodiscard]] nsresult AddAcceptAndDictionaryHeaders(
nsIURI* aURI, ExtContentPolicyType aType, nsHttpRequestHead* aRequest,
- bool aSecure, bool& aAsync,
+ bool aSecure, bool& aAsync, nsHttpChannel* aChan,
+ void (*aSuspend)(nsHttpChannel*),
const std::function<bool(bool, DictionaryCacheEntry*)>& aCallback);
[[nodiscard]] nsresult AddStandardRequestHeaders(
nsHttpRequestHead*, nsIURI* aURI, bool aIsHTTPS,
