Closed Bug 1270753 Opened 8 years ago Closed 8 years ago

Intermittent application crashed [@ mozilla::LinkedList<OldWindowSize>::~LinkedList]

Categories

(Core :: General, defect)

Product:
Core
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla50
Tracking Flags:
Tracking Status
firefox48 --- fixed
firefox49 --- fixed
firefox50 --- fixed

People

(Reporter: cbook, Assigned: xidorn)

References

(
URL
)

Details

(Keywords: crash, intermittent-failure)

Attachments

(2 files)

Bug 1270753 part 1 - Use the weak reference itself as an index for OldWindowSize item.
58 bytes, text/x-review-board-request
smaug
: review+
lizzard
: approval-mozilla-aurora+
gchang
: approval-mozilla-beta+
Details
Bug 1270753 part 2 - Clean up corresponding old window size when the DOMWindowUtils is destroyed.
58 bytes, text/x-review-board-request
smaug
: review+
lizzard
: approval-mozilla-aurora+
Details 
https://treeherder.mozilla.org/logviewer.html#?job_id=27363021&repo=mozilla-inbound#L10141

WARNING -  PROCESS-CRASH | Main app process exited normally | application crashed [@ mozilla::LinkedList<OldWindowSize>::~LinkedList]
 23:28:11     INFO -  Crash dump filename: /tmp/tmp0A0_sv.mozrunner/minidumps/0b99d62d-5ce6-7d21-2c646178-4fee2513.dmp
 23:28:11     INFO -  Operating system: Linux
 23:28:11     INFO -                    0.0.0 Linux 3.2.0-76-generic-pae #111-Ubuntu SMP Tue Jan 13 22:34:29 UTC 2015 i686
 23:28:11     INFO -  CPU: x86
 23:28:11     INFO -       GenuineIntel family 6 model 62 stepping 4
 23:28:11     INFO -       1 CPU
 23:28:11     INFO -  Crash reason:  SIGSEGV
 23:28:11     INFO -  Crash address: 0x0
 23:28:11     INFO -  Process uptime: not available
 23:28:11     INFO -  Thread 0 (crashed)
 23:28:11     INFO -   0  libxul.so!mozilla::LinkedList<OldWindowSize>::~LinkedList [LinkedList.h:0fd722ed257c : 329 + 0xe]
 23:28:11     INFO -      eip = 0xb3616de2   esp = 0xbfc3f4f0   ebp = 0xbfc3f508   ebx = 0xb76c6508
 23:28:11     INFO -      esi = 0xb7705d44   edi = 0xb76c9e80   eax = 0xb5dcc9f6   ecx = 0xb76c6508
 23:28:11     INFO -      edx = 0x00000000   efl = 0x00210282
 23:28:11     INFO -      Found by: given as instruction pointer in context
 23:28:11     INFO -   1  libc-2.15.so + 0x32f2b
 23:28:11     INFO -      eip = 0xb2321f2b   esp = 0xbfc3f510   ebp = 0xb0e2c408   ebx = 0xb2493ff4
 23:28:11     INFO -      esi = 0xb0e2c408   edi = 0xb76c9e80
 23:28:11     INFO -      Found by: call frame info
 23:28:11     INFO -   2  libc-2.15.so + 0x321ee
 23:28:11     INFO -      eip = 0xb23211ee   esp = 0xbfc3f520   ebp = 0xb0e2c408
 23:28:11     INFO -      Found by: stack scanning
 23:28:11     INFO -   3  ld-2.15.so + 0x146b0
 23:28:11     INFO -      eip = 0xb77c36b0   esp = 0xbfc3f538   ebp = 0xbfc3f568
 23:28:11     INFO -      Found by: stack scanning
 23:28:11     INFO -   4  libxul.so!_fini + 0x18
 23:28:11     INFO -      eip = 0xb5daa730   esp = 0xbfc3f570   ebp = 0xbfc3f578
 23:28:11     INFO -      Found by: previous frame's frame pointer
 23:28:11     INFO -   5  ld-2.15.so + 0xf465
 23:28:11     INFO -      eip = 0xb77be465   esp = 0xbfc3f580   ebp = 0xbfc3f7a8
 23:28:11     INFO -      Found by: previous frame's frame pointer
 23:28:11     INFO -   6  libc-2.15.so + 0x32b81
 23:28:11     INFO -      eip = 0xb2321b81   esp = 0xbfc3f7b0   ebp = 0xb24943e4
 23:28:11     INFO -      Found by: previous frame's frame pointer
 23:28:11     INFO -   7  plugin-container + 0x35140
 23:28:11     INFO -      eip = 0x0807d140   esp = 0xbfc3f7c4   ebp = 0xb24943e4
 23:28:11     INFO -      Found by: stack scanning
 23:28:11     INFO -   8  plugin-container!_fini + 0x17da0
 23:28:11     INFO -      eip = 0x08098e5c   esp = 0xbfc3f7c8   ebp = 0xb24943e4
 23:28:11     INFO -      Found by: stack scanning
 23:28:11     INFO -   9  plugin-container!_fini + 0x17b50
 23:28:11     INFO -      eip = 0x08098c0c   esp = 0xbfc3f7cc   ebp = 0xb24943e4
 23:28:11     INFO -      Found by: stack scanning
 23:28:11     INFO -  10  libc-2.15.so + 0x1a4ff4
 23:28:11     INFO -      eip = 0xb2493ff4   esp = 0xbfc3f7e0   ebp = 0xbfc3f7f8
 23:28:11     INFO -      Found by: stack scanning
 23:28:11     INFO -  11  libc-2.15.so + 0x194d3
 23:28:11     INFO -      eip = 0xb23084d3   esp = 0xbfc3f800   ebp = 0x00000001
 23:28:11     INFO -      Found by: previous frame's frame pointer
 23:28:11     INFO -  12  plugin-container!__libc_csu_fini + 0x10
 23:28:11     INFO -      eip = 0x08080fb0   esp = 0xbfc3f804   ebp = 0x00000001
 23:28:11     INFO -      Found by: stack scanning
 23:28:11     INFO -  13  libc-2.15.so + 0x32be6
 23:28:11     INFO -      eip = 0xb2321be6   esp = 0xbfc3f808   ebp = 0x00000001
 23:28:11     INFO -      Found by: stack scanning
 23:28:11     INFO -  14  libc-2.15.so + 0x1a4ff4
 23:28:11     INFO -      eip = 0xb2493ff4   esp = 0xbfc3f80c   ebp = 0x00000001
 23:28:11     INFO -      Found by: stack scanning
 23:28:11     INFO -  15  libc-2.15.so + 0x194db
 23:28:11     INFO -      eip = 0xb23084db   esp = 0xbfc3f810   ebp = 0x00000001
 23:28:11     INFO -      Found by: stack scanning
 23:28:11     INFO -  16  libc-2.15.so + 0x1a4ff4
 23:28:11     INFO -      eip = 0xb2493ff4   esp = 0xbfc3f838   ebp = 0x00000001
 23:28:11     INFO -      Found by: stack scanning
 23:28:11     INFO -  17  plugin-container!_GLOBAL__sub_I_Sandbox.cpp [Sandbox.cpp:0fd722ed257c : 688 + 0x5]
 23:28:11     INFO -      eip = 0x0804fbd4   esp = 0xbfc3f860   ebp = 0x00000001
 23:28:11     INFO -      Found by: stack scanning
23:28:11
Flags: needinfo?(bugzilla)
So this is an assertion from LinkedList that when it is destroyed, it has been empty. And given the LinkedList of OldWindowSize is a static variable, it is destroyed during shutdown.

In theory, this may happen if a window is closed without exiting fullscreen properly. However, it seems the same issue doesn't happen on Windows.

It may only leak if you improperly close a window in fullscreen, and never enter fullscreen again. So the number of leaked object is likely to be very few. And OldWindowSize is also a very small struct. So I don't think this is a big issue.

If it isn't a very annoying intermittent, I tend not to put resource on fixing it.
Flags: needinfo?(bugzilla)
This is also causing most of the failures in bug 1272894.
Probably we can try to do some clean up in the destructor of nsDOMWindowUtils.
Will have a look after I arrive London.
Assignee: nobody → bugzilla
Hmmmm, I guess we don't want to query weak reference in the destructor of nsDOMWindowUtils, since outer window owns the utils, and querying weak reference of the outer window in its destructor could mess things up...
Attachment #8763431 - Flags: review?(bugs) → review+
Comment on attachment 8763431 [details]
Bug 1270753 part 1 - Use the weak reference itself as an index for OldWindowSize item.

https://reviewboard.mozilla.org/r/59602/#review56788

::: dom/base/nsDOMWindowUtils.cpp
(Diff revision 1)
> -  static OldWindowSize* GetItem(nsPIDOMWindowOuter* aWindow)
> +  static OldWindowSize* GetItem(nsIWeakReference* aWindowRef)
>    {
>      OldWindowSize* item = sList.getFirst();
> -    while (item) {
> +    while (item && item->mWindowRef != aWindowRef) {
> -      nsCOMPtr<nsPIDOMWindowOuter> window = do_QueryReferent(item->mWindow);
> -      if (!window) {

it is unclear to me why we remove this list clean up part here. Hopefully the next patch explains
Comment on attachment 8763432 [details]
Bug 1270753 part 2 - Clean up corresponding old window size when the DOMWindowUtils is destroyed.

https://reviewboard.mozilla.org/r/59604/#review56792

ok, this explains it
Attachment #8763432 - Flags: review?(bugs) → review+
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/25517318011e
part 1 - Use the weak reference itself as an index for OldWindowSize item. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/252e74c677b7
part 2 - Clean up corresponding old window size when the DOMWindowUtils is destroyed. r=smaug
https://hg.mozilla.org/mozilla-central/rev/25517318011e
https://hg.mozilla.org/mozilla-central/rev/252e74c677b7
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Please request Aurora approval on this as when you get a chance.
status-firefox49: --- → affected
Flags: needinfo?(bugzilla)
Comment on attachment 8763431 [details]
Bug 1270753 part 1 - Use the weak reference itself as an index for OldWindowSize item.

Approval Request Comment
[Feature/regressing bug #]: bug 1177155
[User impact if declined]: no user impact
[Describe test coverage new/current, TreeHerder]: no test coverage
[Risks and why]: shouldn't be risky as it just changes how the no longer needed items are stripped
[String/UUID change made/needed]: n/a

Sheriffs want this to be uplifted to fix an intermittent shutdown crash on debug build.
Flags: needinfo?(bugzilla)
Attachment #8763431 - Flags: approval-mozilla-aurora?
The uplift request is for both patches in this bug. Uplifting one but not the other may break things.
Comment on attachment 8763431 [details]
Bug 1270753 part 1 - Use the weak reference itself as an index for OldWindowSize item.

Fix for crash on debug build, let's uplift to aurora.
Attachment #8763431 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8763432 [details]
Bug 1270753 part 2 - Clean up corresponding old window size when the DOMWindowUtils is destroyed.

[Triage Comment]
Attachment #8763432 - Flags: approval-mozilla-aurora+
Looks like this affects Beta as well.
status-firefox48: --- → affected
Flags: needinfo?(xidorn+moz)
Comment on attachment 8763431 [details]
Bug 1270753 part 1 - Use the weak reference itself as an index for OldWindowSize item.

Approval Request Comment
[Feature/regressing bug #]: bug 1177155
[User impact if declined]: no user impact
[Describe test coverage new/current, TreeHerder]: no test coverage
[Risks and why]: shouldn't be risky as it just changes how the no longer needed items are stripped
[String/UUID change made/needed]: n/a

Sheriffs want this to be uplifted to fix an intermittent shutdown crash on debug build.
Flags: needinfo?(xidorn+moz)
Attachment #8763431 - Flags: approval-mozilla-beta?
Comment on attachment 8763431 [details]
Bug 1270753 part 1 - Use the weak reference itself as an index for OldWindowSize item.

Take this in 48 beta 7 as this fixes an intermittent shutdown crash on debug build.
Attachment #8763431 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Part one does not apply cleanly to beta.
Flags: needinfo?(xidorn+moz)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: