1965711 - When a StreamFilter breaks while disconnecting, the channel is stuck and the response never stops

Status: NEW

(WebExtensions :: Request Handling, defect, P3)

Description

[Rob Wu [:robwu]]

This bit of logic in StreamFilterParent has a bug: https://searchfox.org/mozilla-central/rev/7a33d9702cfb059f19ece355416b58d6cfbd8093/toolkit/components/extensions/webrequest/StreamFilterParent.cpp#373-385

IPCResult StreamFilterParent::RecvDisconnect() {
  // ...
  mState = State::Disconnecting;
  CheckResult(SendFlushData());
  return IPC_OK();
}
IPCResult StreamFilterParent::RecvFlushedData() {
  // ...
  FinishDisconnect(); // <-- eventually enters Disconnected state and emits OnStopRequest

When state Disconnecting is entered, the intention is to do a roundtrip to the StreamFilterChild, to ensure that any data is flushed from the child to the parent before really disconnecting the StreamFilter. FinishDisconnect() must eventually be triggered.

The logic above calls CheckResult() which calls Broken() to clean up. But Broken() is a no-op if already in the Disconnecting state (otherwise it would just transition to the Disconnecting state + call FinishDisconnect - note that this has a bug: bug 1965710).

So, if SendFlushData() fails, or if the actor is destroyed before the child's SendFlushedData call was received by the parent, then StreamFilterParent stays stuck in the Disconnecting state forever, and OnStopRequest is never called.

P.S. I found this issue based on auditing the code as part of the investigation for bug 1628642. I did not create a test case, but I imagine that this can be reproduced by forcing the extension process to crash before the child can send SendFlushedData.

Comment 1

[Rob Wu [:robwu]]

I added a TODO comment linking this bug in the patch to bug 1628642.