Skip to content

Commit

Permalink
Bug 1633935 - P13 Avoid ODA message being processed when the channel …
Browse files Browse the repository at this point in the history
…is already failed, r=mayhemer,kershaw,necko-reviewers

One line fix with a bunch of explanation.

When we perform networking on the socket process, OnDataAvailable could be sent directly from socket process to child process.
Therefore, it could be racy with OnStartRequest which is from parent process to child process by nature.

This patch is to resolve the failure (cancel) by parent process.
For example, https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/netwerk/protocol/http/nsHttpChannel.cpp#2713

Bug 1641496 introduce a status check to avoid ODA message being process
[2] https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/netwerk/protocol/http/HttpBackgroundChannelChild.cpp#163-171

It prevents putting ODA to the channel event queue in child process.

However, this is based on that `OnStartRequest` already finished excution, which is usually true if OnStartRequest is sent via PHttpChannel.
https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/netwerk/protocol/http/HttpChannelChild.cpp#417-422
ChannelEventQueue::RunOrEnque simply runs the OnStartRequest given there on the same thread and the queue is empty, since no blocking events from different thread are before OnStartRequest.

Now we are moving the next chapter. OnStartRequest is queued given we want to switch thread.
The failure status, which is initiated in parent process, is set in HttpChannelChild::OnStartRequest which could be later than the status check [2]

Therefore, we need another check to prevent ODA, which is already in the event queue, being processed.
We intentionally keep [2] for performance.

Differential Revision: https://phabricator.services.mozilla.com/D79771
  • Loading branch information
JuniorHsu committed Jun 30, 2020
1 parent dcef01a commit 0e217e1
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion netwerk/protocol/http/HttpChannelChild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,7 @@ void HttpChannelChild::OnTransportAndData(const nsresult& aChannelStatus,
return;
}

if (mCanceled) {
if (mCanceled || NS_FAILED(mStatus)) {
return;
}

Expand Down

0 comments on commit 0e217e1

Please sign in to comment.