Crash at nsDeviceContext::FindScreen when running b2g desktop mochitest

RESOLVED FIXED in mozilla38

Status

()

Core
Graphics
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: pchang, Assigned: pchang)

Tracking

Trunk
mozilla38
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 6 obsolete attachments)

(Assignee)

Description

3 years ago
Hit the crash at nsDeviceContext::FindScreen() when test the b2g desktop mochitest with bug 1111433.


07:30:19  WARNING -  PROCESS-CRASH | Main app process exited normally | application crashed [@ nsDeviceContext::FindScreen(nsIScreen**)]
07:30:19     INFO -  Crash dump filename: /tmp/tmpWmb9rs/minidumps/43cf2524-0baf-38a3-3f83bd2b-6f6cbf0a.dmp
07:30:19     INFO -  Operating system: Linux
07:30:19     INFO -                    0.0.0 Linux 3.2.0-23-generic #36-Ubuntu SMP Tue Apr 10 20:39:51 UTC 2012 x86_64
07:30:19     INFO -  CPU: amd64
07:30:19     INFO -       family 6 model 45 stepping 7
07:30:19     INFO -       1 CPU
07:30:19     INFO -  Crash reason:  SIGSEGV
07:30:19     INFO -  Crash address: 0x0
07:30:19     INFO -  Thread 0 (crashed)
07:30:19     INFO -   0  libxul.so!nsDeviceContext::FindScreen(nsIScreen**) [nsDeviceContext.cpp:8ced49e7387a : 645 + 0x0]
07:30:19     INFO -      rbx = 0x00007ff6adb74d40   r12 = 0x0000000000000000
07:30:19     INFO -      r13 = 0x0000000000000213   r14 = 0x0000000000000213
07:30:19     INFO -      r15 = 0x0000000000000001   rip = 0x00007ff6bdc5229b
07:30:19     INFO -      rsp = 0x00007fff8e53db50   rbp = 0x00007fff8e53db98
07:30:19     INFO -      Found by: given as instruction pointer in context
07:30:19     INFO -   1  libxul.so!nsDeviceContext::ComputeFullAreaUsingScreen(nsRect*) [nsDeviceContext.cpp:8ced49e7387a : 619 + 0xa]
07:30:19     INFO -      rbx = 0x00007ff6adb74d40   r12 = 0x00007ff6c01bb260
07:30:19     INFO -      r13 = 0x0000000000000213   r14 = 0x0000000000000213
07:30:19     INFO -      r15 = 0x0000000000000001   rip = 0x00007ff6bdc52320
07:30:19     INFO -      rsp = 0x00007fff8e53db80   rbp = 0x00007fff8e53dbf0
07:30:19     INFO -      Found by: call frame info
...
07:30:19     INFO -  28  libxul.so!mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsCOMArray<mozilla::dom::EventTarget>*) [EventDispatcher.cpp:8ced49e7387a : 634 + 0x4]
07:30:19     INFO -      rbx = 0x00007ff6adb943c0   r12 = 0x00007ff6ad5d9030
07:30:19     INFO -      r13 = 0x0000000000000000   r14 = 0x0000000000000000
07:30:19     INFO -      r15 = 0x00007fff8e54035c   rip = 0x00007ff6be2a1b5b
07:30:19     INFO -      rsp = 0x00007fff8e5401f0   rbp = 0x0000000000000000
07:30:19     INFO -      Found by: call frame info
07:30:19     INFO -  29  libxul.so!mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) [EventDispatcher.cpp:8ced49e7387a : 701 + 0x1a]
07:30:19     INFO -      rbx = 0x00007ff6adb6f700   r12 = 0x00007ff6ad579800
07:30:19     INFO -      r13 = 0x00007ff6ad57a000   r14 = 0x00007fff8e54035c
07:30:19     INFO -      r15 = 0x0000000000000001   rip = 0x00007ff6be2a1e56
07:30:19     INFO -      rsp = 0x00007fff8e5402f0   rbp = 0x00007ff6adb943c0
07:30:19     INFO -      Found by: call frame info
07:30:19     INFO -  30  libxul.so!nsINode::DispatchEvent(nsIDOMEvent*, bool*) [nsINode.cpp:8ced49e7387a : 1272 + 0x4]
07:30:19     INFO -      rbx = 0x00007ff6ad57a000   r12 = 0x00007ff6ad579800
07:30:19     INFO -      r13 = 0x00007ff6adb6f700   r14 = 0x00007fff8e540428
07:30:19     INFO -      r15 = 0x0000000000000001   rip = 0x00007ff6bde18784
07:30:19     INFO -      rsp = 0x00007fff8e540350   rbp = 0x00007fff8e5403a7
07:30:19     INFO -      Found by: call frame info
07:30:19     INFO -  31  libxul.so!mozilla::SelectionCarets::DispatchSelectionStateChangedEvent(mozilla::dom::Selection*, mozilla::dom::Sequence<mozilla::dom::SelectionState> const&) [SelectionCarets.cpp:8ced49e7387a : 1055 + 0x11]
07:30:19     INFO -      rbx = 0x00007ff6adb6f700   r12 = 0x0000000000000001
07:30:19     INFO -      r13 = 0x0000000000000000   r14 = 0x00007fff8e540428
07:30:19     INFO -      r15 = 0x0000000000000001   rip = 0x00007ff6be6f3b30
07:30:19     INFO -      rsp = 0x00007fff8e5403a0   rbp = 0x00007ff6ad579800
07:30:19     INFO -      Found by: call frame info

[callstack source]
https://tbpl.mozilla.org/php/getParsedLog.php?id=54878143&tree=Try&full=1#error5
(Assignee)

Updated

3 years ago
Blocks: 1111433
(Assignee)

Comment 1

3 years ago
From [1], it is not guaranteed that mScreenManager is valid during nsDeviceContext::Init().  

Need to add the null ptr checking for mScreenManager.

[1]http://mxr.mozilla.org/mozilla-central/source/gfx/src/nsDeviceContext.cpp?rev=5081398a2#383
Assignee: nobody → pchang
(Assignee)

Comment 2

3 years ago
Created attachment 8539133 [details] [diff] [review]
check mScreenManager before calling

Check mScreenManager before using.
(Assignee)

Comment 3

3 years ago
Created attachment 8539151 [details] [diff] [review]
v2 check mScreenManager before calling

Add the warning log if mScreenManager is null after nsDeviceContext::Init().
Attachment #8539133 - Attachment is obsolete: true
Attachment #8539151 - Flags: review?(tnikkel)
(Assignee)

Comment 4

3 years ago
(In reply to peter chang[:pchang][:peter] from comment #3)
> Created attachment 8539151 [details] [diff] [review]
> v2 check mScreenManager before calling
> 
> Add the warning log if mScreenManager is null after nsDeviceContext::Init().

tn, the reason why I create this bug is because I hit the crash at nsDeviceContext::FindScreen when running b2g desktop mochitest for bug 1111433. And it only could reproduce in the try server.
(Assignee)

Comment 5

3 years ago
Created attachment 8539156 [details] [diff] [review]
v2 check mScreenManager before calling

Re-upload to correct patch
Attachment #8539151 - Attachment is obsolete: true
Attachment #8539151 - Flags: review?(tnikkel)
Attachment #8539156 - Flags: review?(tnikkel)
Comment on attachment 8539156 [details] [diff] [review]
v2 check mScreenManager before calling

Why are we unable to get a screen manager? That doesn't seem like a minor problem.
(Assignee)

Comment 7

3 years ago
(In reply to Timothy Nikkel (:tn) from comment #6)
> Comment on attachment 8539156 [details] [diff] [review]
> v2 check mScreenManager before calling
> 
> Why are we unable to get a screen manager? That doesn't seem like a minor
> problem.

Is it related to try server configuration? I couldn't reproduce this in local.
(Assignee)

Comment 8

3 years ago
Add more log in nsDeviceContext and push to try again for analysis.

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=d9ae2c68ce68
(Assignee)

Comment 9

3 years ago
Created attachment 8540023 [details] [diff] [review]
v3 check mScreenManager before calling

Add the error checking from do_GetService.
Attachment #8539156 - Attachment is obsolete: true
Attachment #8539156 - Flags: review?(tnikkel)
Attachment #8540023 - Flags: review?(tnikkel)
(Assignee)

Comment 10

3 years ago
tn, from try server result[2], it is possible that there is an error return from do_GetSerivce API.

I think it is reason to add the mScreenManager checking before using.

[Log with null mScreenManager]
07:11:02     INFO -  Init(375) this 0x7fc2d7b4ee00 mScreenManager (nil) mWidget (nil)
07:11:02     INFO -  Init(382) this 0x7fc2d7b4ee00 mScreenManager (nil) mWidget 0x7fc2de8ba820
07:11:02     INFO -  Init(387) this 0x7fc2d7b4ee00 mScreenManager (nil) mWidget 0x7fc2de8ba820
07:11:02     INFO -  FindScreen(650) this 0x7fc2d7b4ee00 mScreenManager (nil) mWidget 0x7fc2de8ba820

[Log with valid mScreenManager]
07:04:02     INFO -  Init(375) this 0x7feca75184a0 mScreenManager (nil) mWidget (nil)
07:04:02     INFO -  Init(382) this 0x7feca75184a0 mScreenManager (nil) mWidget 0x7fecbe403820
07:04:02     INFO -  Init(387) this 0x7feca75184a0 mScreenManager 0x7fecc0b42040 mWidget 0x7fecbe403820
07:04:02     INFO -  FindScreen(650) this 0x7feca75184a0 mScreenManager 0x7fecc0b42040 mWidget 0x7fecbe403820
07:04:02     INFO -  FindScreen(650) this 0x7feca75184a0 mScreenManager 0x7fecc0b42040 mWidget 0x7fecbe403820
07:04:02     INFO -  FindScreen(650) this 0x7feca75184a0 mScreenManager 0x7fecc0b42040 mWidget 0x7fecbe403820


[1]https://tbpl.mozilla.org/?tree=Try&rev=820c47544581
[2]https://tbpl.mozilla.org/php/getParsedLog.php?id=55103439&tree=Try&full=1
Flags: needinfo?(tnikkel)
Since this doesn't seem to fail anywhere else I'm inclined to think that the real bug is that we can't get a screen manager. Can you dig into why that fails?
Flags: needinfo?(tnikkel)
(Assignee)

Comment 12

3 years ago
(In reply to Timothy Nikkel (:tn) from comment #11)
> Since this doesn't seem to fail anywhere else I'm inclined to think that the
> real bug is that we can't get a screen manager. Can you dig into why that
> fails?

tn, I got the error during the initialization of screen manager from another try run.
Any suggestion for the next debug step?
NS_ERROR_FACTORY_NOT_REGISTERED (0x80040154)
    Returned when a service could not be found.
Flags: needinfo?(tnikkel)
Are we running this code too early during startup before some required thing is initialized? Or too late during shutdown after something required has shutdown? Can you find the code that is returning the error and see why it is returning an error?
Flags: needinfo?(tnikkel)
(Assignee)

Comment 14

3 years ago
(In reply to Timothy Nikkel (:tn) from comment #13)
> Are we running this code too early during startup before some required thing
> is initialized? Or too late during shutdown after something required has
> shutdown? Can you find the code that is returning the error and see why it
> is returning an error?

The error is returning from do_GetService in [1]. I guess it may be the FindScreen calls maybe too late during the shutdown stage because the patch from bug 1111433 will send out a blur event(DOM event)[2] from BrowserElementChild to shell.js via BrowserElementParent.
Maybe I can use the shutdown flag[3] in BrowserElementChildPreload.js to identify the timing issue.

[1]http://dxr.mozilla.org/mozilla-central/source/gfx/src/nsDeviceContext.cpp?from=nsDeviceContext.cpp&case=true#383 
[2]http://dxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementChildPreload.js#674
[3]http://dxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementChildPreload.js#276
Have you had any luck identifying the timing issue?
(Assignee)

Comment 16

3 years ago
(In reply to Timothy Nikkel (:tn) from comment #15)
> Have you had any luck identifying the timing issue?

Now I can reproduce the null ScreenManager after running the folloing mochitest under layout/base/test

[test_event_target_iframe_oop.html]

And it can get the ScreenManager if I remove the oop flag in [1] and [2].

[1]http://dxr.mozilla.org/mozilla-central/source/layout/base/tests/test_event_target_iframe_oop.html#117
[2]http://dxr.mozilla.org/mozilla-central/source/layout/base/tests/test_event_target_iframe_oop.html#125
(Assignee)

Comment 17

3 years ago
Finally I found the possible root cause related this mochitest crash.

During running the test_event_target_iframe_oop.html, it has high possibility that several BrowserElementChildPreload.js are loaded after shutdown processing. (I think those page creation are related to the SimpleTest Framework.)

In the bug 1111433, I add the patch to send out the DOM event when pages get blur. But those pages in [1] doesn't create the windows[2], I guess this is the reason why crash because of screenmanager is not register in comment 12.

[1]http://pastebin.mozilla.org/8167635
[2]http://dxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementChildPreload.js#741
(Assignee)

Comment 18

3 years ago
I will add '_isContentWindowCreated' checking before sending out DOM event[1] in bug 1111433 to prevent crash because of un-registered screenmanager service.

[1]http://dxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementChildPreload.js#596
(Assignee)

Comment 19

3 years ago
Created attachment 8545116 [details] [diff] [review]
v4 check mScreenManager before calling

Dump the error code if fail to get the screen manager service.
Attachment #8540023 - Attachment is obsolete: true
Attachment #8540023 - Flags: review?(tnikkel)
Attachment #8545116 - Flags: review?(tnikkel)
(Assignee)

Comment 20

3 years ago
tn, I already update the patch in bug 1111433 but I think it is better to handle the null screen manager inside nsDeviceContext. Please feel free to let me know your comment.
(Assignee)

Comment 21

3 years ago
Comment on attachment 8545116 [details] [diff] [review]
v4 check mScreenManager before calling

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

::: gfx/src/nsDeviceContext.cpp
@@ +387,5 @@
> +    nsresult rv;
> +    mScreenManager = do_GetService("@mozilla.org/gfx/screenmanager;1", &rv);
> +
> +    if (NS_FAILED(rv)) {
> +        NS_WARNING("Failed to get screenmanager service: %x", rv);

oops...looks like NS_WARNING doesn't accept 2 arguments to dump nsresult. Later, I will change to other log output
(Assignee)

Comment 22

3 years ago
Created attachment 8545713 [details] [diff] [review]
v5 check mScreenManager before calling

Fix the build error and return function result to the caller of nsDeviceContext::Init().
Attachment #8545116 - Attachment is obsolete: true
Attachment #8545116 - Flags: review?(tnikkel)
Attachment #8545713 - Flags: review?(tnikkel)
(Assignee)

Updated

3 years ago
No longer blocks: 1111433
Attachment #8545713 - Flags: review?(tnikkel) → review+
(Assignee)

Comment 23

3 years ago
Created attachment 8552138 [details] [diff] [review]
v5 check mScreenManager before calling

Update reviewer into patch
Attachment #8545713 - Attachment is obsolete: true
Attachment #8552138 - Flags: review+
(Assignee)

Comment 24

3 years ago
The try looks good.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a2a4687ef9f
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/71ad6315d88c
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
(Assignee)

Comment 27

3 years ago
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #25)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/71ad6315d88c
> 
> Please include your name and email address in the commit information of
> future patches.
> https://developer.mozilla.org/en-US/docs/
> Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-
> in_for_me.3F

Thanks for the help, I will check my setting for generating the patch
You need to log in before you can comment on or make changes to this bug.