Skip to content

Commit

Permalink
Bug 1870499 - Make sure ContinueDoNotifyListener is called immediatel…
Browse files Browse the repository at this point in the history
…y during release a=diannaS

The causes of the runnable loop are as follows:
1. In Release we have a special case to make sure OnStart&OnStop are always
   called. We do this by dispatching a runnable that calls DoNotifyListener.
2. DoNotifyListener is called, then the reference to the channel is again
   dropped. But at this point only OnStartRequest was called, and
   ContinueDoNotifyListener was added to mEventQ. That means another
   runnable would be dispatched.

While the loop was broken in the previous patch, we still want to ensure
OnStopRequest is called before the channel is released, so this patch
adds an argument to DoNotifyListener to select whether ContinueDoNotifyListener
should be called synchronously or dispatched to mEventQ.

Original Revision: https://phabricator.services.mozilla.com/D202798

Differential Revision: https://phabricator.services.mozilla.com/D204032
  • Loading branch information
valenting committed Mar 8, 2024
1 parent 8af00dd commit ebaf186
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 7 deletions.
16 changes: 10 additions & 6 deletions netwerk/protocol/http/HttpChannelChild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ NS_IMETHODIMP_(MozExternalRefCountType) HttpChannelChild::Release() {
RefPtr<HttpChannelChild> channel = dont_AddRef(this);
NS_DispatchToMainThread(NS_NewRunnableFunction(
"~HttpChannelChild>DoNotifyListener",
[chan = std::move(channel)] { chan->DoNotifyListener(); }));
[chan = std::move(channel)] { chan->DoNotifyListener(false); }));
// If NS_DispatchToMainThread failed then we're going to leak the runnable,
// and thus the channel, so there's no need to do anything else.
return mRefCnt;
Expand Down Expand Up @@ -1433,7 +1433,7 @@ void HttpChannelChild::NotifyOrReleaseListeners(nsresult rv) {
DoNotifyListener();
}

void HttpChannelChild::DoNotifyListener() {
void HttpChannelChild::DoNotifyListener(bool aUseEventQueue) {
LOG(("HttpChannelChild::DoNotifyListener this=%p", this));
MOZ_ASSERT(NS_IsMainThread());

Expand All @@ -1452,10 +1452,14 @@ void HttpChannelChild::DoNotifyListener() {
}
StoreOnStartRequestCalled(true);

mEventQ->RunOrEnqueue(new NeckoTargetChannelFunctionEvent(
this, [self = UnsafePtr<HttpChannelChild>(this)] {
self->ContinueDoNotifyListener();
}));
if (aUseEventQueue) {
mEventQ->RunOrEnqueue(new NeckoTargetChannelFunctionEvent(
this, [self = UnsafePtr<HttpChannelChild>(this)] {
self->ContinueDoNotifyListener();
}));
} else {
ContinueDoNotifyListener();
}
}

void HttpChannelChild::ContinueDoNotifyListener() {
Expand Down
4 changes: 3 additions & 1 deletion netwerk/protocol/http/HttpChannelChild.h
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,9 @@ class HttpChannelChild final : public PHttpChannelChild,
const ResourceTimingStructArgs& timing);
void Redirect3Complete();
void DeleteSelf();
void DoNotifyListener();
// aUseEventQueue should only be false when called from
// HttpChannelChild::Release to make sure OnStopRequest is called syncly.
void DoNotifyListener(bool aUseEventQueue = true);
void ContinueDoNotifyListener();
void OnAfterLastPart(const nsresult& aStatus);
void MaybeConnectToSocketProcess();
Expand Down

0 comments on commit ebaf186

Please sign in to comment.