Last Comment Bug 770218 - Shutdown crash in DOMSVGTransformList::IsAnimValList with deterministicgc() and watchpoints
: Shutdown crash in DOMSVGTransformList::IsAnimValList with deterministicgc() a...
: crash, testcase
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: -- critical (vote)
: mozilla17
Assigned To: Andrew McCreight [:mccr8]
: Jet Villegas (:jet)
Depends on:
Blocks: 326633
  Show dependency treegraph
Reported: 2012-07-02 09:26 PDT by Jesse Ruderman
Modified: 2012-07-23 09:17 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (causes a shutdown crash) (309 bytes, text/html)
2012-07-02 09:26 PDT, Jesse Ruderman
no flags Details
stack trace (2.58 KB, text/plain)
2012-07-02 09:27 PDT, Jesse Ruderman
no flags Details
null check in case of double unlink (1.77 KB, patch)
2012-07-17 12:04 PDT, Andrew McCreight [:mccr8]
jwatt: review+
Details | Diff | Splinter Review

Description Jesse Ruderman 2012-07-02 09:26:49 PDT
Created attachment 638390 [details]
testcase (causes a shutdown crash)

1. Install
2. Run Firefox
3. Load the testcase.
4. Quit Firefox.

Result: shutdown crash [@ mozilla::DOMSVGTransformList::IsAnimValList]

Likely tickled by the current leakiness of deterministicgc(), which is bug 769015. But this seems more serious than a leak.
Comment 1 Jesse Ruderman 2012-07-02 09:27:14 PDT
Created attachment 638391 [details]
stack trace
Comment 2 Jesse Ruderman 2012-07-02 09:32:02 PDT
Might be debug-only.
Comment 3 Andrew McCreight [:mccr8] 2012-07-02 10:25:05 PDT
The immediate cause is a double-unlink problem: when you unlink DOMSVGTransformList twice, you end up doing a null-deref.  That's easy enough to fix.

But it does seem odd that the GC and the CC aren't agreeing on whether the transform list is alive or not...
Comment 4 Andrew McCreight [:mccr8] 2012-07-02 10:33:46 PDT
This probably isn't too big a deal.  The GC thinking something is alive that the CC thinks is dead causes double-unlinking at worst.  The SVGTransformList is being held alive via a watchpoint, so I suspect some watchpoint-related shenanigans.
Comment 5 Andrew McCreight [:mccr8] 2012-07-03 09:10:50 PDT
Talking to Bill, I think the problem is just the GC not running during shutdown in this mode.  That would explain the GC weirdness, making this not really a security bug.  I'm going to wait for him to confirm that before opening this up.  I'll just use this bug to fix the double unlink issue.
Comment 6 Andrew McCreight [:mccr8] 2012-07-17 12:04:52 PDT
Created attachment 643070 [details] [diff] [review]
null check in case of double unlink
Comment 7 Andrew McCreight [:mccr8] 2012-07-18 06:53:15 PDT
Try run looked good.
Comment 8 Jonathan Watt [:jwatt] 2012-07-18 07:01:51 PDT
Comment on attachment 643070 [details] [diff] [review]
null check in case of double unlink

DOMSVGNumberList.cpp and DOMSVGTransformList.cpp should also get the same treatment. r=jwatt with that.
Comment 9 Andrew McCreight [:mccr8] 2012-07-18 15:46:06 PDT
Thanks for pointing out the additional classes!  I fixed all 3 in that directory.  The other unlink functions looked okay to me.
Comment 10 Ed Morley [:emorley] 2012-07-19 07:32:29 PDT
Comment 11 Andrew McCreight [:mccr8] 2012-07-23 09:17:41 PDT
I don't see any crashes like the stack trace here, so it probably isn't worth backporting.

Note You need to log in before you can comment on or make changes to this bug.