Closed Bug 1578919 Opened 5 years ago Closed 5 years ago

SW parent_intercept: Crash in [@ mozilla::ipc::InputStreamHelper::PostSerializationActivation]

Categories

(Core :: DOM: Service Workers, defect, P2)

Unspecified
Linux
defect

Tracking

()

VERIFIED FIXED
mozilla71
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox69 --- unaffected
firefox70 --- unaffected
firefox71 blocking verified

People

(Reporter: asuth, Assigned: perry)

References

(Regression)

Details

(Keywords: crash, regression, topcrash, Whiteboard: rca - AutoIPCStream design error)

Crash Data

Attachments

(3 files)

This bug is for crash report bp-59542135-db39-44db-8f5a-694d60190904.

Top 10 frames of crashing thread:

0 libxul.so mozilla::ipc::InputStreamHelper::PostSerializationActivation ipc/glue/InputStreamUtils.cpp:272
1 libxul.so mozilla::ipc:: ipc/glue/IPCStreamUtils.cpp:228
2 libxul.so mozilla::ipc::AutoIPCStream::~AutoIPCStream ipc/glue/IPCStreamUtils.cpp
3 libxul.so mozilla::dom::ServiceWorkerPrivateImpl::PendingFetchEvent::~PendingFetchEvent dom/serviceworkers/ServiceWorkerPrivateImpl.cpp:783
4 libxul.so mozilla::dom::ServiceWorkerPrivateImpl::PendingFetchEvent::~PendingFetchEvent dom/serviceworkers/ServiceWorkerPrivateImpl.cpp:777
5 libxul.so nsTArray_Impl<mozilla::UniquePtr<mozilla::dom::ServiceWorkerPrivateImpl::PendingFunctionalEvent, mozilla::DefaultDelete<mozilla::dom::ServiceWorkerPrivateImpl::PendingFunctionalEvent> >, nsTArrayInfallibleAllocator>::Clear mfbt/UniquePtr.h:486
6 libxul.so mozilla::dom::ServiceWorkerPrivateImpl::UpdateState dom/serviceworkers/ServiceWorkerPrivateImpl.cpp:970
7 libxul.so mozilla::dom::ServiceWorkerPrivate::UpdateState dom/serviceworkers/ServiceWorkerPrivate.cpp:1867
8 libxul.so mozilla::dom::ServiceWorkerInfo::UpdateState dom/serviceworkers/ServiceWorkerInfo.cpp:156
9 libxul.so mozilla::dom::ServiceWorkerRegistrationInfo::FinishActivate dom/serviceworkers/ServiceWorkerRegistrationInfo.cpp:369

This looks like it might be a use-after-free because the assertion about an unexpected InputStreamParams union type already has complete coverage, which suggests the allocator poisoned the memory when it performed the free.

This may have involved devtools inspecting a ServiceWorker with the debugger paused on processing a fetch event. Or it might not have.

We have a spike in this signature starting in 20191009095806 - macOS and Windows are affected. Other than the Linux crash Andrew reported in 20190903094847, there was only 2 additonal crashes in 71, in 20190914212452 and 20190927094817.

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=035f52aed4427b22facfa883067e298f10ef9e97&tochange=0c108c06ee6e3173a22578d00bc0f02ab0fb9e31

Some SW stuff is in that changeset - might that have caused the spike? ni on perry

Flags: needinfo?(perry)

I experience a crash with this signature when opening Google News. Here is one of my crash reports:
https://crash-stats.mozilla.org/report/index/901dd542-9271-47a6-850e-a39050191010

Setting dom.serviceWorkers.parent_intercept to false and restarting Firefox seems to "fix" the crash.

Regressed by: 1456995

Blocker for 71 given the volume.

Hello! It seems that I found some easy STR for this crash:

  1. Open Firefox (dom.serviceWorkers.parent_intercept:true) and https://news.google.com/?hl=en&gl=US&ceid=US:en
  2. Middle-click multiple links from the site in order to open them in multiple tabs.

AR: Browser crashes (if not repeat the above steps).
It seems that with pref on after middle clicking some links the browser starts lagging very bad and then crashes.

Assignee: nobody → perry
Flags: needinfo?(perry)

Even easier STR: https://tv.bell.ca/ always causes the crash for me.

In ServiceWorkerPrivateImpl::SendFetchEvent, a heap-allocated AutoIPCStream can
point to a stack-allocated IPCStream (part of an IPCInternalRequest). If this
IPCStream is destroyed before the AutoIPCStream, the AutoIPCStream will have a
dangling pointer (and this is the case if SendFetchEvent is called when the
Service Worker's state is "activating" rather than "activated").

This patch moves around the logic to handle the AutoIPCStream's lifetime to
ensure it its lifetime is within its IPCStream's lifetime. The larger issue
might be that AutoIPCStream doesn't have inherent lifetime guarantees (it'll
definitely outlive its IPCStream if it points to its embedded one, but it
doesn't own any external IPCStreams it might point to).

Depends on D48934

This patch factors out some of the initial test case in
fetch-waits-for-activate.https.html (which only tests "pending" fetch events
for a navigation request) to share with a new test case that tests
"pending" fetch events for a subresource request with a request body.

Both tests in the file have a high-level structure of:

  1. Register a Service Worker and wait until its state is "activating" but don't
    let its state reach "activated".
  2. Fire a fetch event that will be controlled by that Service Worker, which
    should wait until the Service Worker's state advances to "activated".
  3. Wait for the fetch to see that the worker isn't "activated". This step isn't
    directly observable by content, so the test's method to determine this can
    have false posities (but should never cause the test to unexpectedly fail).
  4. Tell the Service Worker to advance to "activated".
  5. Verify the fetch that was dispatched while the Service Worker was
    "activating" is successfully handled.

Depends on D48935

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/19653 for changes under testing/web-platform/tests

WPT sync failing due to testing additional test against mozilla-central, which hasn't received the non-test changes. Will re-trigger once all changesets have landed on mozilla-central.

Flags: needinfo?(perry)

Hello! This particular issue is verified fixed following the STR from comment 5 on Windows 10x64, macOS 10.14 and Ubuntu 18.04.
While verifying this I encountered bug 1588357 and posted the steps to reproduce here, althought is pretty intermittent.

Status: RESOLVED → VERIFIED
Upstream PR merged by jgraham

I just had another Nightly crash with this signature. Crash report UID is 75e6618a-01a1-4d8a-a1e0-76d700191011. In my understanding this shouldn't be possible anymore?

Yesterday also two crashes. So three crashes in the last two days.

Hello TMart!
Are you using the latest Nightly build?
I also checked for this crash signature and did not see any crashes on the builds that contain the fix.

Flags: needinfo?(Tobias.Marty)

Sorry for the confusion, these crashes are with older builds. Nightly just crashed for me but didn't leave a crash report. That's what's gotten me confused.

Flags: needinfo?(Tobias.Marty)
Crash Signature: [@ mozilla::ipc::InputStreamHelper::PostSerializationActivation] → [@ mozilla::ipc::InputStreamHelper::PostSerializationActivation] [@ mozilla::ipc::InputStreamHelper::PostSerializationActivation(mozilla::ipc::InputStreamParams&, bool, bool)]

This bug has been identified as part of a pilot on determining root causes of blocking and dot release drivers.

It needs a root-cause set for it. Please see the list at https://docs.google.com/document/d/1FFEGsmoU8T0N8R9kk-MXWptOPtXXXRRIe4vQo3_HgMw/.

Add the root cause as a whiteboard tag in the form [rca - <cause> ] and remove the rca-needed keyword.

If you have questions, please contact :tmaity.

Keywords: rca-needed
Crash Signature: [@ mozilla::ipc::InputStreamHelper::PostSerializationActivation] [@ mozilla::ipc::InputStreamHelper::PostSerializationActivation(mozilla::ipc::InputStreamParams&, bool, bool)] → [@ mozilla::ipc::InputStreamHelper::PostSerializationActivation] [@ mozilla::ipc::InputStreamHelper::PostSerializationActivation(mozilla::ipc::InputStreamParams&, bool, bool)]
Keywords: rca-needed
Whiteboard: rca - AutoIPCStream design error
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: