Closed Bug 1138916 Opened 10 years ago Closed 10 years ago

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

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla39
Tracking Flags:
Tracking Status
firefox39 --- fixed

People

(Reporter: opatinobugzilla, Assigned: bkelly)

References

Details

(Whiteboard: service workers cache)

Attachments

(2 files)

backtrace_segfault_firefox_sw.txt
(deleted), text/plain
Details
Fix Cache::MatchAll() assertion in WorkerFeature handling. r=baku
(deleted), patch
baku
: review+
Details | Diff | Splinter Review
Attached file backtrace_segfault_firefox_sw.txt (deleted) —
Browser crashes when service worker calls cache.matchAll without parameters. These parameters are optional so it can be call without any.
Whiteboard: service workers cache
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.
Assignee: nobody → bkelly
Blocks: 940273
Component: DOM: Workers → DOM
Status: NEW → ASSIGNED
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.
Attachment #8571979 - Flags: review?(amarchesini)
Note, I verified this test triggers the reported assertion as well.
Attachment #8571979 - Flags: review?(amarchesini) → review+
Keywords: checkin-needed
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f3d517d39e43
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox39: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Now matchAll without parameters is working. Thanks bkelly.
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: