Shutdown hang due to JavaScript error: resource:///modules/sessionstore/SessionStore.jsm, line 1627: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIObserverService.removeObserver]

RESOLVED FIXED in Firefox 63

Status

()

defect
P1
major
RESOLVED FIXED
10 months ago
9 months ago

People

(Reporter: whimboo, Assigned: mikedeboer)

Tracking

59 Branch
Firefox 64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed, firefox64 fixed)

Details

Attachments

(1 attachment)

This Javascript failure can be seen a lot in Marionette test jobs and which seem to be related to a shutdown hang of Firefox:

https://treeherder.mozilla.org/logviewer.html#?job_id=197591760&repo=mozilla-inbound&lineNumber=53890-53895

> task 2018-09-05T12:42:01.319Z] 12:42:01     INFO -  JavaScript error: resource:///modules/sessionstore/SessionStore.jsm, line 1627: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIObserverService.removeObserver]
> [task 2018-09-05T12:42:01.336Z] 12:42:01     INFO -  1536151321324	Marionette	DEBUG	Closed connection 1
> [task 2018-09-05T12:42:01.412Z] 12:42:01     INFO -  ###!!! [Parent][RunMessage] Error: Channel closing: too late to send/recv, messages will be lost
> [task 2018-09-05T12:42:02.162Z] 12:42:02     INFO -  1536151322158	Marionette	DEBUG	Received observer notification xpcom-will-shutdown
> [task 2018-09-05T12:42:02.163Z] 12:42:02     INFO -  1536151322159	Marionette	DEBUG	Remote service is inactive
> [task 2018-09-05T12:48:02.461Z] 12:48:02     INFO - TEST-UNEXPECTED-ERROR | js/xpconnect/tests/marionette/test_loader_global_sharing.py TestLoaderGlobalSharing.test_global_sharing_settings | IOError: Process killed because the connection to Marionette server is lost. Check gecko.log for errors (Reason: [Errno 111] Connection refused)

The problem is inside `AsyncShutdown.quitApplicationGranted.addBlocker`:

https://dxr.mozilla.org/mozilla-central/rev/c2e3be6a1dd352b969a45f0b85e87674e24ad284/browser/components/sessionstore/SessionStore.jsm#1627

As it looks like the custom timer as setup with `waitTimeMaxMs` doesn't fire and also the background hang monitor doesn't crash Firefox. As such the process hangs around infinitely until it gets killed.

This can be best seen for bug 1444600 which started to happen more frequently on August 19th:

https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?startday=2018-08-16&endday=2018-09-10&tree=all&bug=1444600

And that mostly for Linux64 ASAN builds.

Mike or Adam, can one of you see if that is related? If not, maybe we could simply fix this failure to get it out of possible candidates? Thanks.
Flags: needinfo?(mdeboer)
Summary: avaScript error: resource:///modules/sessionstore/SessionStore.jsm, line 1627: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIObserverService.removeObserver] → JavaScript error: resource:///modules/sessionstore/SessionStore.jsm, line 1627: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIObserverService.removeObserver]
Assignee

Comment 1

10 months ago
Henrik, did you mean to n-i Adam too?

So this code does check for abnormal shutdowns to bail out of the shutdown blocker. If a syntax error like this occurs there, this logic will fail and the blocker will remain active.
I'll submit a patch to address this.
Flags: needinfo?(mdeboer)
Assignee

Updated

10 months ago
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Priority: -- → P1
(In reply to Mike de Boer [:mikedeboer] from comment #1)
> Henrik, did you mean to n-i Adam too?

Sorry, looks like I missed that.

> blocker. If a syntax error like this occurs there, this logic will fail and
> the blocker will remain active.

That would explain those shutdown hangs! Thanks for the patch in advance.
Summary: JavaScript error: resource:///modules/sessionstore/SessionStore.jsm, line 1627: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIObserverService.removeObserver] → Shutdown hang due to JavaScript error: resource:///modules/sessionstore/SessionStore.jsm, line 1627: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIObserverService.removeObserver]
Comment on attachment 9008719 [details]
Bug 1489960 - Ensure to not throw errors inside the SessionStore shutdown blocker, to continue the sequence. r?mconley!

Mike Conley (:mconley) (:⚙️) has approved the revision.
Attachment #9008719 - Flags: review+

Comment 5

10 months ago
Pushed by mdeboer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/80144c8b5d4e
Ensure to not throw errors inside the SessionStore shutdown blocker, to continue the sequence. r=mconley

Comment 6

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/80144c8b5d4e
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Please request Beta approval on this when you get a chance.
Flags: needinfo?(mdeboer)
Assignee

Comment 8

9 months ago
Comment on attachment 9008719 [details]
Bug 1489960 - Ensure to not throw errors inside the SessionStore shutdown blocker, to continue the sequence. r?mconley!

Approval Request Comment
[Feature/Bug causing the regression]: Increasing amount of test failure during shutdown in Marionette. This should help lowering the failure rate.
[User impact if declined]: This may also fix a shutdown hang, session restore failure in rare cases. But this was fixed mainly to help our infra.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: No, it has merely landed on Nightly and stuck.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: N/a.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Because we're catching errors now, instead of throwing them, which is always better.
[String changes made/needed]: N/a.
Flags: needinfo?(mdeboer)
Attachment #9008719 - Flags: approval-mozilla-beta?
Comment on attachment 9008719 [details]
Bug 1489960 - Ensure to not throw errors inside the SessionStore shutdown blocker, to continue the sequence. r?mconley!

Approved for 63 Beta 8, thanks.
Attachment #9008719 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.