trained-to-thrill Service Worker crashes child process on FirefoxOS

RESOLVED FIXED in Firefox 38

Status

()

Core
DOM
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: bkelly, Assigned: baku)

Tracking

unspecified
mozilla38
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(firefox38 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
Using this patch queue (build available soon):

  https://github.com/wanderview/gecko-patches/tree/serviceworkers

I see the child process crash when visiting trained-to-thrill:

  https://jakearchibald.github.io/trained-to-thrill/

Note, it happens on the second visit when fetch events start getting processed.

This could be related to bug 1133238 where we see a CORS failure, but I'm not sure.
(Reporter)

Comment 1

3 years ago
Is this something the Paris folks have already solved?  Am I missing a patch from that queue?
Flags: needinfo?(amarchesini)
(Reporter)

Comment 2

3 years ago
This seems to effect mdn.github.io/sw-test as well.
(Reporter)

Comment 3

3 years ago
I didn't see anything in logcat.  Maybe a debug build would show more.  Unfortunately I don't have time to investigate now.
I am hitting this crash as well. I can see this in the logcat:

 208  Gecko  I  [Parent 208] WARNING: Principal is invalid, killing app process: file ../../../dom/ipc/AppProcessChecker.cpp, line 222
 208  Gecko  I  [Parent 208] WARNING: 'mActor && !AssertAppPrincipal(mActor, principal)', file ../../../dom/cache/PrincipalVerifier.cpp, line 137
 208  Gecko  I  [Parent 208] WARNING: 'NS_FAILED(aRv)', file ../../../dom/cache/CacheStorageParent.cpp, line 225
(Reporter)

Comment 5

3 years ago
Thanks Fernando!  I wonder if the appId being passed is garbage or something.
(Reporter)

Comment 6

3 years ago
I believe Fernando and Andrea are investigating this.  It does seem the principal is incorrect.  They tell me the security check is seeing:

  appId 0 and inBrowserElement false

But logcat indicates the browser has these values instead:

  AppId 32 inBrowserElement 1

So the ServiceWorker is getting the wrong principal somehow.
Some more information. It seems that the principal is wrong when we get to WorkerGlobalScope::GetCaches which seems to be the starting point for the Cache API.

30657 Gecko  I  ServiceWorkerManager::Register. documentPrincipal. appId 32 inBrowser 1
30657 Gecko  I  ServiceWorkerRegisterJob::Start. principal. appId 32
30657 Gecko  I  ServiceWorkerRegisterJob::Start mJobType == REGISTER_JOB
30657 Gecko  I  ServiceWorkerRegisterJob::Start mRegistration
30657 Gecko  I  StoreRegistration 32
30657 Gecko  I  PopulateRegistrationData appId 32
29882 Gecko  I  AppId 32 inBrowserElement 1
29882 Gecko  I  OwnOrContainingAppId 32

30657 Gecko  I  WorkerGlobalScope::GetCaches appId 0
30657 Gecko  I  CacheStorage::CreateOnWorker
30657 Gecko  I  CacheStorage::CacheStorage
30657 Gecko  I  CacheStorage::ActorCreated
29882 Gecko  I  CacheStorageParent::CacheStorageParent appId 0
30657 Gecko  I  CacheStorage::Open
29882 Gecko  I  VerifyOnMainThread()
29882 Gecko  I  AppId 0 inBrowserElement 0
29882 Gecko  I  OwnOrContainingAppId 32
29882 Gecko  I  [Parent 29882] WARNING: Principal is invalid, killing app process: file ../../../dom/ipc/AppProcessChecker.cpp, line 224
29882 Gecko  I  [Parent 29882] WARNING: 'mActor && !AssertAppPrincipal(mActor, principal)', file ../../../dom/cache/PrincipalVerifier.cpp, line 138
29882 Gecko  I  [Parent 29882] WARNING: 'NS_FAILED(aRv)', file ../../../dom/cache/CacheStorageParent.cpp, line 228
I can also reproduce this issue with http://ferjm.github.io/bug1133242/index.html The sw code is at https://github.com/ferjm/bug1133242/blob/master/sw.js
(Assignee)

Updated

3 years ago
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
(Assignee)

Comment 9

3 years ago
https://mxr.mozilla.org/mozilla-central/source/dom/workers/ServiceWorkerManager.cpp#2182

This is the reason why we have this crash.
(Assignee)

Comment 10

3 years ago
Created attachment 8565154 [details] [diff] [review]
principal.patch
Attachment #8565154 - Flags: review?(nsm.nikhil)
Comment on attachment 8565154 [details] [diff] [review]
principal.patch

Review of attachment 8565154 [details] [diff] [review]:
-----------------------------------------------------------------

Have you verified that this fixes the problem?
Attachment #8565154 - Flags: review?(nsm.nikhil) → review+
FWIW the patch fixes the issue for me. Thank you Andrea!
(Assignee)

Comment 13

3 years ago
Created attachment 8565333 [details] [diff] [review]
229391.diff

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e6484423f16d
Attachment #8565154 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
(Assignee)

Comment 14

3 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1295c57ba35c
(Assignee)

Comment 15

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/32e74ad032ea
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/32e74ad032ea
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.