Closed Bug 714562 Opened 13 years ago Closed 13 years ago

Intermittent Assertion failure: info.prevp, at /builds/slave/m-in-osx64-dbg/build/js/src/jsgc.cpp:779 or crash [@ js::gc::Chunk::releaseArena]

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
x86_64
macOS
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla12

People

(Reporter: philor, Assigned: igor)

References

Details

(Keywords: assertion, intermittent-failure, regression)

Attachments

(1 file, 2 obsolete files)

v3
8.56 KB, patch
billm
: review+
Details | Diff | Splinter Review 
Yet another thing I've been hoping was also other things, like bug 714545. This is from the push after the fix for it landed, but there have been a few before then, too.

https://tbpl.mozilla.org/php/getParsedLog.php?id=8269807&tree=Mozilla-Inbound
Rev4 MacOSX Snow Leopard 10.6 mozilla-inbound debug test mochitest-other on 2012-01-01 15:57:29 PST for push b76de21ba602

TEST-INFO | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_bug557956.js | Console message: LOG addons.xpi: Install of http://example.com/browser/toolkit/mozapps/extensions/test/browser/browser_bug557956_8_2.xpi completed.
Assertion failure: info.prevp, at /builds/slave/m-in-osx64-dbg/build/js/src/jsgc.cpp:779
WARNING: shutting down early because of crash!: file /builds/slave/m-in-osx64-dbg/build/dom/plugins/ipc/PluginModuleChild.cpp, line 746
WARNING: plugin process _exit()ing: file /builds/slave/m-in-osx64-dbg/build/dom/plugins/ipc/PluginModuleChild.cpp, line 711
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_bug557956.js | Exited with code 1 during test run
INFO | automation.py | Application ran for: 0:26:47.399499
INFO | automation.py | Reading PID log: /var/folders/Hs/HsDn6a9SG8idoIya6p9mtE+++TI/-Tmp-/tmp3mzFPvpidlog
PROCESS-CRASH | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_bug557956.js | application crashed (minidump found)
Crash dump filename: /var/folders/Hs/HsDn6a9SG8idoIya6p9mtE+++TI/-Tmp-/tmpc_U7Dw/minidumps/F328A31B-C076-41E9-8AD9-017419A487BE.dmp
Operating system: Mac OS X
                  10.6.8 10K549
CPU: amd64
     family 6 model 23 stepping 10
     2 CPUs

Crash reason:  EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
Crash address: 0x0

Thread 0 (crashed)
 0  XUL!CrashInJS [jsutil.cpp:b76de21ba602 : 95 + 0x0]
    rbx = 0x261a4000   r12 = 0x049f1400   r13 = 0x051c3508   r14 = 0x006660d0
    r15 = 0x049f1610   rip = 0x0101ae04   rsp = 0x5fbfae40   rbp = 0x5fbfae40
    Found by: given as instruction pointer in context
 1  XUL!js::gc::Chunk::releaseArena [jsgc.cpp:b76de21ba602 : 779 + 0x17]
    rbx = 0x261a4000   r12 = 0x049f1400   r13 = 0x051c3508   r14 = 0x006660d0
    r15 = 0x049f1610   rip = 0x0309e3fd   rsp = 0x5fbfae50   rbp = 0x5fbfaed0
    Found by: call frame info
 2  XUL!js::gc::FinalizeArenas [jsgc.cpp:b76de21ba602 : 362 + 0xe]
    rbx = 0x261a4000   r12 = 0x480f2008   r13 = 0x0000000c   r14 = 0x006660d0
    r15 = 0x049f1610   rip = 0x030a0152   rsp = 0x5fbfaee0   rbp = 0x5fbfaf50
    Found by: call frame info
 3  XUL!SweepPhase [jsgc.cpp:b76de21ba602 : 1516 + 0x16]
    rbx = 0x04caae10   r12 = 0x049f1400   r13 = 0x04caaed0   r14 = 0x04caaed0
    r15 = 0x006660d0   rip = 0x030adffc   rsp = 0x5fbfaf60   rbp = 0x5fbfafc0
    Found by: call frame info
 4  XUL!GCCycle [jsgc.cpp:b76de21ba602 : 2829 + 0x18]
    rbx = 0x022f1f40   r12 = 0x3d456400   r13 = 0x05183000   r14 = 0x00000000
    r15 = 0x00000000   rip = 0x030af152   rsp = 0x5fbfafd0   rbp = 0x5fbfb170
    Found by: call frame info
 5  XUL!js_GC [jsgc.cpp:b76de21ba602 : 3125 + 0xe]
    rbx = 0x05183000   r12 = 0x006660d0   r13 = 0x5fbfb1a0   r14 = 0x00000000
    r15 = 0x1601fd01   rip = 0x030afa0f   rsp = 0x5fbfb180   rbp = 0x5fbfb1f0
    Found by: call frame info
 6  XUL!nsXPConnect::Collect [nsXPConnect.cpp:b76de21ba602 : 412 + 0x7]
    rbx = 0x05d7d000   r12 = 0x006660d0   r13 = 0x5fbfb220   r14 = 0x00000000
    r15 = 0x1601fd53   rip = 0x022bce51   rsp = 0x5fbfb200   rbp = 0x5fbfb320
    Found by: call frame info
 7  XUL!nsXPConnect::GarbageCollect [nsXPConnect.cpp:b76de21ba602 : 420 + 0xc]
    rbx = 0x0085c200   r12 = 0x00000000   r13 = 0x01c62530   r14 = 0x00000000
    r15 = 0x1601fd53   rip = 0x022b8c51   rsp = 0x5fbfb330   rbp = 0x5fbfb330
    Found by: call frame info
 8  XUL!nsJSContext::GarbageCollectNow [nsJSEnvironment.cpp:b76de21ba602 : 3243 + 0xc]
    rbx = 0x0085c200   r12 = 0x00000000   r13 = 0x01c62530   r14 = 0x00000000
    r15 = 0x1601fd53   rip = 0x01c6246d   rsp = 0x5fbfb340   rbp = 0x5fbfb350
    Found by: call frame info
 9  XUL!nsTimerImpl::Fire [nsTimerImpl.cpp:b76de21ba602 : 428 + 0xa]
    rbx = 0x00000002   r12 = 0x4ba76ba0   r13 = 0x01c62530   r14 = 0x00000000
    r15 = 0x1601fd53   rip = 0x02c6769b   rsp = 0x5fbfb360   rbp = 0x5fbfb3e0
This is yet another regression from the bug 702251 that is not covered by the fix in the bug 714344. I have missed that during the decommit outside the GC lock the allocation thread may have allocated all the arenas from the chunk and removed it from the available list. In this case the chunk should be added back to the available list.
Attached patch v1 (obsolete) — — Splinter Review 
This is what I have sent to the try server - the patch effectively replaces that Maybe<AutoUnlock> fix from the 714344 with a code to properly remove/add chunk from the available list. It does not look too complex/fragile as I initially thought it would be in the bug 714344.
Assignee: general → igor
Summary: Intermittent Assertion failure: info.prevp, at /builds/slave/m-in-osx64-dbg/build/js/src/jsgc.cpp:779 → Intermittent Assertion failure: info.prevp, at /builds/slave/m-in-osx64-dbg/build/js/src/jsgc.cpp:779 or crash [@ js::gc::Chunk::releaseArena]
Attachment #585248 - Flags: review?(wmccloskey)
Attached patch v2 (obsolete) — — Splinter Review 
In the new version I made comments clear.
Attachment #585248 - Attachment is obsolete: true
Attachment #585248 - Flags: review?(wmccloskey)
Attachment #585276 - Flags: review?(wmccloskey)
Attached patch v3 — — Splinter Review 
The new version skips unlocking during the decommit if the main thread already waits in GC for the background thread to finish. This helps to avoids extra system calls resulting from unlock/lock calls on a contested lock and lets the main thread to start the GC faster.
Attachment #585276 - Attachment is obsolete: true
Attachment #585276 - Flags: review?(wmccloskey)
Attachment #585685 - Flags: review?(wmccloskey)
Comment on attachment 585685 [details] [diff] [review]
v3

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

This seems correct. Eventually it would be nice if we could refactor the code so that we're not duplicating the logic from allocateArena and releaseArena here.

This list manipulation is getting pretty gross. Do you think it would be easier if the prev pointers pointed to chunks instead of to their next pointers? It might be worth a try.

::: js/src/jsgc.cpp
@@ +2315,5 @@
> +                        insertPoint = availableListHeadp;
> +                }
> +                chunk->insertToAvailableList(insertPoint);
> +            } else {
> +                JS_ASSERT(chunk->info.prevp);

Can you assert !chunk->unused() here? That way this code matches up more clearly with the releaseArena code.
Attachment #585685 - Flags: review?(wmccloskey) → review+
(In reply to Bill McCloskey (:billm) from comment #13)
> This list manipulation is getting pretty gross. Do you think it would be
> easier if the prev pointers pointed to chunks instead of to their next
> pointers?

With that the first chunk can not just point to a field in JSRuntime * requiring more if checks when adding or removing chunks to the list. Another complication is that just from the value of info.prevp it would be impossible to tell if the chunk is on the available list or not. That would have to be replaced with something like chunk->hasAvailableArenas() && !chunk->unused() bringing complications when updating the counters.

In any case, at some point I want to replace the chunks list with a sorted vector as that it seems allows for lesser fragmentation.
https://hg.mozilla.org/mozilla-central/rev/8062086f8e00
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
Blocks: 702251
Keywords: regression
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: