Closed
Bug 832609
Opened 10 years ago
Closed 10 years ago
test_823965.html : Assertion failure: (next == this) == (prev == this), at ../../../dist/include/mozilla/LinkedList.h:191
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
People
(Reporter: jfkthame, Assigned: justin.lebar+bug)
References
(Blocks 1 open bug)
Details
(Keywords: intermittent-failure)
Attachments
(2 files, 1 obsolete file)
1020 bytes,
patch
|
justin.lebar+bug
:
review+
lsblakk
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
sicking
:
approval-mozilla-b2g18+
|
Details | Diff | Splinter Review |
4.08 KB,
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
https://tbpl.mozilla.org/php/getParsedLog.php?id=18943119&tree=Mozilla-Inbound Assertion failure: (next == this) == (prev == this), at ../../../dist/include/mozilla/LinkedList.h:191 TEST-UNEXPECTED-FAIL | /tests/dom/devicestorage/test/test_823965.html | Exited with code 11 during test run PROCESS-CRASH | /tests/dom/devicestorage/test/test_823965.html | application crashed [@ mozilla::LinkedListElement<nsDOMMemoryFile::DataOwner>::isInList() const] Thread 31 (crashed) Operating system: Linux 0.0.0 Linux 2.6.31.5-127.fc12.x86_64 #1 SMP Sat Nov 7 21:11:14 EST 2009 x86_64 CPU: amd64 family 6 model 23 stepping 10 2 CPUs Crash reason: SIGSEGV Crash address: 0x0 Thread 31 (crashed) 0 libxul.so!mozilla::LinkedListElement<nsDOMMemoryFile::DataOwner>::isInList() const [LinkedList.h : 191 + 0x18] rbx = 0x000000000b07f720 r12 = 0x0000000000000000 r13 = 0x0000000000000001 r14 = 0x00007f1d6aafecc0 r15 = 0x0000000000000000 rip = 0x00007f1dccbd2f9e rsp = 0x00007f1d6aafeb10 rbp = 0x00007f1d6aafeb10 Found by: given as instruction pointer in context 1 libxul.so!mozilla::LinkedListElement<nsDOMMemoryFile::DataOwner>::~LinkedListElement [LinkedList.h : 120 + 0x4] rbx = 0x000000000b07f720 r12 = 0x0000000000000000 r13 = 0x0000000000000001 r14 = 0x00007f1d6aafecc0 r15 = 0x0000000000000000 rip = 0x00007f1dccbd3039 rsp = 0x00007f1d6aafeb20 rbp = 0x00007f1d6aafeb30 Found by: call frame info 2 libxul.so!nsDOMMemoryFile::DataOwner::Release() [nsDOMFile.h : 399 + 0x7] rbx = 0x000000000b07f720 r12 = 0x0000000000000000 r13 = 0x0000000000000001 r14 = 0x00007f1d6aafecc0 r15 = 0x0000000000000000 rip = 0x00007f1dccbee8f9 rsp = 0x00007f1d6aafeb40 rbp = 0x00007f1d6aafeb50 Found by: call frame info 3 libxul.so!DataOwnerAdapter::Release() [nsDOMFile.cpp : 87 + 0x4] rbx = 0x0000000002c04be0 r12 = 0x0000000000000000 r13 = 0x0000000000000001 r14 = 0x00007f1d6aafecc0 r15 = 0x0000000000000000 rip = 0x00007f1dccbee9d8 rsp = 0x00007f1d6aafeb60 rbp = 0x00007f1d6aafeb70 Found by: call frame info 4 libxul.so!nsTArray_Impl<nsCOMPtr<nsIInputStream>, nsTArrayInfallibleAllocator>::DestructRange(unsigned int, unsigned int) [nsTArray.h : 432 + 0x4] rbx = 0x0000000003244ab0 r12 = 0x0000000003244ab0 r13 = 0x0000000000000001 r14 = 0x00007f1d6aafecc0 r15 = 0x0000000000000000 rip = 0x00007f1dcd843657 rsp = 0x00007f1d6aafeb80 rbp = 0x00007f1d6aafeb90 Found by: call frame info 5 libxul.so!nsTArray_Impl<nsCOMPtr<nsIInputStream>, nsTArrayInfallibleAllocator>::RemoveElementsAt(unsigned int, unsigned int) [nsTArray.h : 1017 + 0xc] rbx = 0x0000000000000000 r12 = 0x0000000003e682b8 r13 = 0x0000000000000001 r14 = 0x00007f1d6aafecc0 r15 = 0x0000000000000000 rip = 0x00007f1dcd8438e3 rsp = 0x00007f1d6aafeba0 rbp = 0x00007f1d6aafebc0 Found by: call frame info ...etc.
Comment 1•10 years ago
|
||
Can this LinkedList be used by multiple threads?
Yeah, it definitely can be ...
Assignee | ||
Comment 3•10 years ago
|
||
...without locking? What's happening here is that the linked list destructor is running, and it's checking whether the node is in a list. In this case, it's partially in a list (one of the pointer is to self, but the other pointer is not). That's only a valid state if the list is currently being mutated. Which means that either we have memory corruption or the list is being mutated /while the destructor is running/.
Well DataOwner's dtor can run on any thread, so if there's no locking we need to add some.
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee: nobody → khuey
Status: NEW → ASSIGNED
Attachment #705443 -
Flags: review?(justin.lebar+bug)
Assignee | ||
Comment 7•10 years ago
|
||
khuey and I talked about this on IRC, and we agree this patch is scary. So what we're going to do is disable this memory reporter on b2g18 (that's safest possible change we could make there) and then try a simpler approach for trunk (register one memory reporter for each large blob). This simpler approach requires bug 833913.
Attachment #705443 -
Attachment is obsolete: true
Attachment #705443 -
Flags: review?(justin.lebar+bug)
Attachment #705531 -
Flags: review?(justin.lebar+bug)
Assignee | ||
Comment 9•10 years ago
|
||
Comment on attachment 705531 [details] [diff] [review] Disable the reporter on b2g18. [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 803684 User impact if declined: crashes, memory corruption, bad times Testing completed: Looks about right to me. Risk to taking this patch (and alternatives if risky): Hard to imagine things getting worse as a result of this patch. String or UUID changes made by this patch: none
Attachment #705531 -
Flags: review?(justin.lebar+bug)
Attachment #705531 -
Flags: review+
Attachment #705531 -
Flags: approval-mozilla-b2g18?
Attachment #705531 -
Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
Assignee | ||
Updated•10 years ago
|
Whiteboard: checkin-needed
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Whiteboard: checkin-needed
https://hg.mozilla.org/releases/mozilla-b2g18/rev/3f59ebb61ca9
status-b2g18:
--- → fixed
Keywords: checkin-needed
Comment on attachment 705531 [details] [diff] [review] Disable the reporter on b2g18. [Approval Request Comment] See comment 9.
Attachment #705531 -
Flags: approval-mozilla-beta?
Attachment #705531 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Attachment #705531 -
Flags: approval-mozilla-beta?
Attachment #705531 -
Flags: approval-mozilla-beta+
Attachment #705531 -
Flags: approval-mozilla-aurora?
Attachment #705531 -
Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-beta/rev/1b7a0f44689c https://hg.mozilla.org/releases/mozilla-aurora/rev/f4f89f5b0d0f
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 15•10 years ago
|
||
Aand we never followed up with this bug. :(
Assignee | ||
Comment 17•10 years ago
|
||
The paper-over fix needs to be uplifted to Aurora again, and we need to actually fix this. It's not just the dtor; the constructor also can run on any thread, afaict from bug 873623.
Assignee | ||
Comment 18•10 years ago
|
||
Comment on attachment 705531 [details] [diff] [review] Disable the reporter on b2g18. We need to re-land this on Aurora. :(
Attachment #705531 -
Flags: approval-mozilla-aurora+ → approval-mozilla-aurora?
Ah crap. I hate it when I lose track of bugs.
Assignee | ||
Comment 20•10 years ago
|
||
I'm compiling a patch for this that involves making a StaticMutex, which is something I've wanted in many other cases. If this works, it would be pretty nice...
Assignee | ||
Updated•10 years ago
|
Assignee: khuey → justin.lebar+bug
Assignee | ||
Comment 21•10 years ago
|
||
No way this will compile anywhere else. https://tbpl.mozilla.org/?tree=Try&rev=6469459a5db5
Assignee | ||
Comment 22•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=c7d94b3376ac
Assignee | ||
Comment 23•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=807ae33a65b1
Assignee | ||
Comment 24•10 years ago
|
||
Depends on bug 873801, which has its own dependency.
Attachment #751407 -
Flags: review?(khuey)
Attachment #751407 -
Flags: review?(khuey) → review+
Updated•10 years ago
|
Comment 25•10 years ago
|
||
Comment on attachment 705531 [details] [diff] [review] Disable the reporter on b2g18. Approving for Aurora 23 - should we also get this on Beta 22? We aren't tracking this bug because it's not clear that we have signs of significant, reproducible user impact here. Please nominate if you believe it to be more serious.
Attachment #705531 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 26•10 years ago
|
||
> We aren't tracking this bug because it's not clear that we have signs of significant, > reproducible user impact here. Probably because few sites use workers. > should we also get this on Beta 22? Yes; thanks for asking. > Please nominate if you believe it to be more serious. I believe this to be pretty serious. I'll elaborate in a PM.
No longer depends on: 833913
Assignee | ||
Comment 27•10 years ago
|
||
Comment on attachment 705531 [details] [diff] [review] Disable the reporter on b2g18. Re-nom'ing for beta. It's a shame we missed this for the last release, but that ship has sailed.
Attachment #705531 -
Flags: approval-mozilla-beta+ → approval-mozilla-beta?
Assignee | ||
Comment 28•10 years ago
|
||
Checkin-needed to re-land on Aurora (and beta, if and when we get re-approval).
Keywords: checkin-needed
Whiteboard: [for checkin-needed, see comment 28]
Comment 29•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/729fb1f47da8 Not sure what the beta holdup is here...
Keywords: checkin-needed
Whiteboard: [for checkin-needed, see comment 28]
Updated•10 years ago
|
tracking-firefox22:
--- → ?
Comment 30•10 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM][Out May 23-27] from comment #29) > https://hg.mozilla.org/releases/mozilla-aurora/rev/729fb1f47da8 > > Not sure what the beta holdup is here... It's not clear that this intermittent failure is hitting FF22, so we were waiting for jlebar to elaborate on severity here (didn't get a PM).
Assignee | ||
Comment 31•10 years ago
|
||
I sent a PM to lsblakk immediately after posting the comment. I didn't get a response; maybe it didn't go through. I'll just say it here: I think you could exploit this bug without a lot of difficulty.
Updated•10 years ago
|
Attachment #705531 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•10 years ago
|
tracking-firefox23:
--- → +
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 32•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8d0ac9d38a1b checkin-needed for mozilla-beta still stands.
Assignee | ||
Comment 33•10 years ago
|
||
Gah, https://hg.mozilla.org/integration/mozilla-inbound/rev/56f89cd46261 landed with this bug in the commit message instead of bug 873801. At least they're related bugs...
Comment 34•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/56f89cd46261 https://hg.mozilla.org/mozilla-central/rev/8d0ac9d38a1b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Updated•10 years ago
|
Assignee | ||
Comment 35•10 years ago
|
||
Ah, that's why this hasn't landed on beta: The m-c merge removed checkin-needed. I'll land this on beta myself.
Comment 36•10 years ago
|
||
That and the whole holiday weekend thing...
Assignee | ||
Comment 37•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/48a67a635157
Updated•10 years ago
|
I just hit the following assertion: http://pastebin.mozilla.org/2565047 It was my understanding that this bug should have fixed it.
That's not an assertion, that's just a stack. I bet that's a refcounting threadsafety assertion firing.
Well, now it's an assertion + a stack:) http://pastebin.mozilla.org/2565061 And yes, it looks like a refcounting threadsafety assertion.
Yeah my guess is that we create it on whatever thread wants to use it first which may not be the main thread. It should really just have threadsafe refcounting. Open a new bug please.
You need to log in
before you can comment on or make changes to this bug.
Description
•