1138916 - [service workers][cache]Seg Fault when calling cache.matchAll without parameters

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Oscar Patiño González]

Attachment: backtrace_segfault_firefox_sw.txt

Browser crashes when service worker calls cache.matchAll without parameters. These parameters are optional so it can be call without any.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Thanks Oscar!

I believe I know what this is.  When I added the WorkerFeature to Cache I used assertions to check that a feature is only added once to each actor.  This doesn't quite hold for the stream control actor because each one manages multiple streams.

The tests did not catch this because the matchAll() is likely done on main thread and not worker.  There is a separate bug to improve test coverage.

I'll have a patch shortly.

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Fix Cache::MatchAll() assertion in WorkerFeature handling. r=baku

Andrea,

There is a small bug in the WorkerFeature code I wrote for Cache.  I assumed each actor would only ever get a Feature added once.  This doesn't hold for the CacheStreamControlChild, though, as it may be controlling multiple ReadStream objects.

This patch fixes the issue by permitting a duplicate SetFeature() as long as the later Feature objects are identical to the first Feature.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Note, I verified this test triggers the reported assertion as well.

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c5969349d9a1

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/f3d517d39e43

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f3d517d39e43

Comment 7

[Oscar Patiño González]

Now matchAll without parameters is working. Thanks bkelly.