Closed
Bug 1430508
Opened 7 years ago
Closed 7 years ago
Check that channel IPC isn't closed before creating StreamFilter endpoints
Categories
(WebExtensions :: Request Handling, defect, P2)
WebExtensions
Request Handling
Tracking
(firefox-esr52 unaffected, firefox57 wontfix, firefox58 fixed, firefox59 fixed)
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox57 | --- | wontfix |
firefox58 | --- | fixed |
firefox59 | --- | fixed |
People
(Reporter: kmag, Assigned: kmag)
Details
Attachments
(1 file)
(deleted),
text/x-review-board-request
|
dragana
:
review+
jcristau
:
approval-mozilla-release+
|
Details |
We sometimes wind up in corner cases where we attempt to attach a StreamFilter to a channel after its IPC is closed. I'm not entirely sure how this happens, but it seems to only happen when devtools network filtering is enabled.
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8942548 [details]
Bug 1430508: Return 0 for ProcessId() when channel IPC is closed.
https://reviewboard.mozilla.org/r/212792/#review218482
Will you try to investigate why request listeners haven't been notified?
Attachment #8942548 -
Flags: review?(dd.mozilla) → review+
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to Dragana Damjanovic [:dragana] from comment #2)
> Will you try to investigate why request listeners haven't been notified?
Yes. I've looked into it a bit, but I can't get rr working with Firefox at the moment, so it's hard for me to be sure of all of the details. I think we're winding up with the actor being destroyed because of something like a content process shutdown before the response has started:
https://searchfox.org/mozilla-central/rev/137f1b2f434346a0c3756ebfcbdbee4069e15dc8/netwerk/protocol/http/HttpChannelParent.cpp#106-120
But the cleanup that we do in that case is pretty trivial, so the channel doesn't get closed right away.
Updated•7 years ago
|
Priority: -- → P2
Assignee | ||
Comment 4•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/06f766e7640a3cd180f7da7c092823e500a3d674
Bug 1430508: Return 0 for ProcessId() when channel IPC is closed. r=dragana
Comment 5•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment 6•7 years ago
|
||
Is manual testing required on this bug? If Yes, please provide some STR and the proper webextension(if required), if No set the “qe-verify-“ flag.
Flags: needinfo?(kmaglione+bmo)
Assignee | ||
Comment 7•7 years ago
|
||
(In reply to marius.santa from comment #6)
> Is manual testing required on this bug? If Yes, please provide some STR and
> the proper webextension(if required), if No set the “qe-verify-“ flag.
Probably not. I tested myself as best as I could, but it's hard to reliably reproduce. I don't think additional manual testing would be worth the effort.
Flags: needinfo?(kmaglione+bmo) → qe-verify-
Assignee | ||
Comment 8•7 years ago
|
||
Comment on attachment 8942548 [details]
Bug 1430508: Return 0 for ProcessId() when channel IPC is closed.
Approval Request Comment
[Feature/Bug causing the regression]: Bug 1255894
[User impact if declined]: This bug causes the main browser process to crash in certain corner cases when extensions attach response filters to requests. I've only been able to produce a crash when attaching filters very late in the request cycle, so extensions can avoid the crash by avoiding that pattern, but I'd still rather have it fixed.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: No.
[Needs manual test from QE? If yes, steps to reproduce]: No, I've verified it locally, which I think should be enough in this case.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: It's a very simple change that checks for the condition that would cause a crash and turns it into a non-fatal error.
[String changes made/needed]: None.
Attachment #8942548 -
Flags: approval-mozilla-beta?
Updated•7 years ago
|
status-firefox57:
--- → wontfix
status-firefox58:
--- → affected
status-firefox-esr52:
--- → unaffected
Comment 9•7 years ago
|
||
Comment on attachment 8942548 [details]
Bug 1430508: Return 0 for ProcessId() when channel IPC is closed.
crash fix for 58 rc2.
Attachment #8942548 -
Flags: approval-mozilla-beta? → approval-mozilla-release+
Comment 10•7 years ago
|
||
bugherder uplift |
Comment 11•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/a91d21ef7ef2 (FIREFOX_58b_RELBRANCH)
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•