Bug 1940471 - Fix race in StreamFilter between ODA and FinishDisconnect r=rpl

For the analysis, see https://bugzilla.mozilla.org/show_bug.cgi?id=1940471#c2

When OnDataAvailable is called after FinishDisconnect has called
FlushBufferedData(), but before mState was updated to mDisconnected,
then OnDataAvailable did not know that it could write the data.
Consequently, OnDataAvailable proceeded to buffer the data. If
OnDataAvailable is not called again (which flushes the buffer if
needed), then this data would be lost forever (which was caught in debug
builds as an assertion failure - see bug report).

This patch introduces a new flag to ensure that OnDataAvailable writes
data immediately instead of buffering. With this, there are now three
flags that help OnDataAvailable to short-circuit early:
- mDisconnectedByOnStartRequest (set on main thread)
- mState == State::Disconnected (set on actor thread)
- mDisconnectedByFinishDisconnect (set on IO thread - new)

Differential Revision: https://phabricator.services.mozilla.com/D244921
This commit is contained in:
Rob Wu
2025-04-10 16:33:16 +00:00
parent c090de99f8
commit e10a67faf8
2 changed files with 12 additions and 1 deletions

View File

@@ -389,6 +389,7 @@ IPCResult StreamFilterParent::RecvFlushedData() {
void StreamFilterParent::FinishDisconnect() {
RefPtr<StreamFilterParent> self(this);
RunOnIOThread(FUNC, [=] {
self->mDisconnectedByFinishDisconnect = true;
self->FlushBufferedData();
RunOnMainThread(FUNC, [=] {
@@ -700,7 +701,8 @@ StreamFilterParent::OnDataAvailable(nsIRequest* aRequest,
uint64_t aOffset, uint32_t aCount) {
AssertIsIOThread();
if (mDisconnectedByOnStartRequest || mState == State::Disconnected) {
if (mDisconnectedByOnStartRequest || mDisconnectedByFinishDisconnect ||
mState == State::Disconnected) {
// If we're offloading data in a thread pool, it's possible that we'll
// have buffered some additional data while waiting for the buffer to
// flush. So, if there's any buffered data left, flush that before we

View File

@@ -182,6 +182,15 @@ class StreamFilterParent final : public PStreamFilterParent,
// to late to be set, which leads out of sync.
bool mDisconnectedByOnStartRequest = false;
// When in mState Disconnecting, data from ODA is buffered, and flushed when
// FinishDisconnect() is called. After this, mState is set to Disconnected,
// and any subsequent data to ODA are written directly to mOrigListener.
// Between flushing the data (on the IO thread) and updating mState (on the
// actor thread), it is theoretically possible to receive another ODA (on the
// IO thread). This flag here ensures that ODA knows that data should be
// written instead of buffered.
bool mDisconnectedByFinishDisconnect = false;
bool mBeforeOnStartRequest = true;
nsCOMPtr<nsISupports> mContext;