Last Comment Bug 656947 - Crash in canvas code with print() while busy [@ JS_updateMallocCounter]
: Crash in canvas code with print() while busy [@ JS_updateMallocCounter]
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: Canvas: 2D (show other bugs)
: Other Branch
: All All
: -- normal (vote)
: ---
Assigned To: Matt Woodrow (:mattwoodrow)
:
:
Mentors:
Depends on:
Blocks: 591358 658440 711397
  Show dependency treegraph
 
Reported: 2011-05-13 10:08 PDT by Alon Zakai (:azakai)
Modified: 2011-12-16 08:41 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
fixed
-
fixed


Attachments
NULL check the current canvas context (1.24 KB, patch)
2011-05-16 14:45 PDT, Matt Woodrow (:mattwoodrow)
bzbarsky: review+
bugzilla: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Alon Zakai (:azakai) 2011-05-13 10:08:49 PDT
The following code will consistently crash Firefox,

<html>
  <head>
    <script type="text/javascript">
      function onLoad() {
        var N = 1000;
        var x = 0;
        for (var i = 0; i < N; i++) {
          for (var j = 0; j < N; j++)
            x += i-j;
          print('waka');
        }
      }
    </script>
  </head>
  <body onload="onLoad()">
    <canvas id="canvas" width="400" height="300"></canvas>
  </body>
</html>

STR: Load the page, press 'ok' when asked to prevent showing further dialogs.

It crashes because nsCanvasRenderingContext2D calls JS_updateMallocCounter with NULL for the context. Backtrace:

Program received signal SIGSEGV, Segmentation fault.
JS_updateMallocCounter (cx=0x0, nbytes=480000) at /home/alon/Dev/mozilla-central/js/src/jsapi.cpp:2020
2020	    return cx->runtime->updateMallocCounter(nbytes);
(gdb) where
#0  JS_updateMallocCounter (cx=0x0, nbytes=480000) at /home/alon/Dev/mozilla-central/js/src/jsapi.cpp:2020
#1  0xb5f57a65 in nsCanvasRenderingContext2D::SetDimensions (this=0xa4c38a60, width=400, height=300)
    at /home/alon/Dev/mozilla-central/content/canvas/src/nsCanvasRenderingContext2D.cpp:1128
#2  0xb6044367 in nsHTMLCanvasElement::UpdateContext (this=0xa4c21e80, aNewContextOptions=0x0)
    at /home/alon/Dev/mozilla-central/content/html/content/src/nsHTMLCanvasElement.cpp:609
#3  0xb6044bab in nsHTMLCanvasElement::GetContext (this=0xa4c21e80, aContextId=..., aContextOptions=..., aContext=0xbfffdadc)
    at /home/alon/Dev/mozilla-central/content/html/content/src/nsHTMLCanvasElement.cpp:536
#4  0xb604346f in nsHTMLCanvasElement::CopyInnerTo (this=0xab02a5e0, aDest=0xa4c21e80)
    at /home/alon/Dev/mozilla-central/content/html/content/src/nsHTMLCanvasElement.cpp:158
#5  0xb60439b8 in nsHTMLCanvasElement::Clone (this=0xab02a5e0, aNodeInfo=0xad0bd710, aResult=0xbfffdbd8)
    at /home/alon/Dev/mozilla-central/content/html/content/src/nsHTMLCanvasElement.cpp:101
#6  0xb5ebb84e in nsNodeUtils::CloneAndAdopt (aNode=0xab02a5e0, aClone=1, aDeep=1, aNewNodeInfoManager=0xa4c57a90, aCx=0x0, aNewScope=0x0, 
    aNodesWithProperties=..., aParent=0xa4cdf680, aResult=0xbfffdc70) at /home/alon/Dev/mozilla-central/content/base/src/nsNodeUtils.cpp:518
#7  0xb5ebb460 in nsNodeUtils::CloneAndAdopt (aNode=0xab028a80, aClone=1, aDeep=1, aNewNodeInfoManager=0xa4c57a90, aCx=0x0, aNewScope=0x0, 
    aNodesWithProperties=..., aParent=0xa4c21ac0, aResult=0xbfffdd10) at /home/alon/Dev/mozilla-central/content/base/src/nsNodeUtils.cpp:664
#8  0xb5ebb460 in nsNodeUtils::CloneAndAdopt (aNode=0xab02a340, aClone=1, aDeep=1, aNewNodeInfoManager=0xa4c57a90, aCx=0x0, aNewScope=0x0, 
    aNodesWithProperties=..., aParent=0xa4c4d400, aResult=0xbfffddb0) at /home/alon/Dev/mozilla-central/content/base/src/nsNodeUtils.cpp:664
#9  0xb5ebb460 in nsNodeUtils::CloneAndAdopt (aNode=0xace40800, aClone=1, aDeep=1, aNewNodeInfoManager=0x0, aCx=0x0, aNewScope=0x0, 
    aNodesWithProperties=..., aParent=0x0, aResult=0xbfffde88) at /home/alon/Dev/mozilla-central/content/base/src/nsNodeUtils.cpp:664
#10 0xb5ebce3d in nsNodeUtils::CloneAndAdopt (aNode=0xace40800, aDeep=1, aCallUserDataHandlers=0, aResult=0xbfffdf74)
    at /home/alon/Dev/mozilla-central/content/base/src/nsNodeUtils.h:301
#11 nsNodeUtils::Clone (aNode=0xace40800, aDeep=1, aCallUserDataHandlers=0, aResult=0xbfffdf74)
    at /home/alon/Dev/mozilla-central/content/base/src/nsNodeUtils.h:175
#12 nsNodeUtils::CloneNodeImpl (aNode=0xace40800, aDeep=1, aCallUserDataHandlers=0, aResult=0xbfffdf74)
    at /home/alon/Dev/mozilla-central/content/base/src/nsNodeUtils.cpp:439
#13 0xb5e2ba7f in nsDocument::CloneNode (this=0xace40800, aDeep=1, aReturn=0xbfffdf74)
    at /home/alon/Dev/mozilla-central/content/base/src/nsDocument.cpp:5779
#14 0xb5e4e615 in nsIDocument::CreateStaticClone (this=0xace40800, aCloneContainer=0xab0d9498)
    at /home/alon/Dev/mozilla-central/content/base/src/nsDocument.cpp:8097
#15 0xb66d4869 in nsPrintObject::Init (this=0xa4c90a40, aDocShell=0xadf46498, aDoc=0xace408f8, aPrintPreview=0)
    at /home/alon/Dev/mozilla-central/layout/printing/nsPrintObject.cpp:120
#16 0xb66cffe4 in nsPrintEngine::DoCommonPrint (this=0xa4c909c0, aIsPrintPreview=0, aPrintSettings=0xabac6240, aWebProgressListener=0x0, 
    aDoc=0xace408f8) at /home/alon/Dev/mozilla-central/layout/printing/nsPrintEngine.cpp:543
#17 0xb66d145a in nsPrintEngine::CommonPrint (this=0xa4c909c0, aIsPrintPreview=0, aPrintSettings=0xabac6240, aWebProgressListener=0x0, 
    aDoc=0xace408f8) at /home/alon/Dev/mozilla-central/layout/printing/nsPrintEngine.cpp:444
#18 0xb66d18fb in nsPrintEngine::Print (this=0xa4c909c0, aPrintSettings=0xabac6240, aWebProgressListener=0x0)
    at /home/alon/Dev/mozilla-central/layout/printing/nsPrintEngine.cpp:759
#19 0xb59f7dd5 in DocumentViewerImpl::Print (this=0xace6d830, aPrintSettings=0xabac6240, aWebProgressListener=0x0)
    at /home/alon/Dev/mozilla-central/layout/base/nsDocumentViewer.cpp:3669
#20 0xb59ec94e in DocumentViewerImpl::LoadComplete (this=0xace6d830, aStatus=0)
    at /home/alon/Dev/mozilla-central/layout/base/nsDocumentViewer.cpp:1086
#21 0xb6ae7bae in nsDocShell::EndPageLoad (this=0xadf46400, aProgress=0xadf46414, aChannel=0xae08e8a0, aStatus=0)
    at /home/alon/Dev/mozilla-central/docshell/base/nsDocShell.cpp:6057
#22 0xb6afcb21 in nsDocShell::OnStateChange (this=0xadf46400, aProgress=0xadf46414, aRequest=0xae08e8a0, aStateFlags=131088, aStatus=0)
    at /home/alon/Dev/mozilla-central/docshell/base/nsDocShell.cpp:5917
#23 0xb6b1b8e6 in nsDocLoader::FireOnStateChange (this=0xadf46400, aProgress=0xadf46414, aRequest=0xae08e8a0, aStateFlags=131088, aStatus=0)
    at /home/alon/Dev/mozilla-central/uriloader/base/nsDocLoader.cpp:1339
#24 0xb6b1bb06 in nsDocLoader::doStopDocumentLoad (this=0xadf46400, request=0xae08e8a0, aStatus=0)
    at /home/alon/Dev/mozilla-central/uriloader/base/nsDocLoader.cpp:947
#25 0xb6b1c79e in nsDocLoader::DocLoaderIsEmpty (this=0xadf46400, aFlushLayout=1)
    at /home/alon/Dev/mozilla-central/uriloader/base/nsDocLoader.cpp:823
#26 0xb6b1d038 in nsDocLoader::OnStopRequest (this=0xadf46400, aRequest=0xace74b40, aCtxt=0x0, aStatus=0)
    at /home/alon/Dev/mozilla-central/uriloader/base/nsDocLoader.cpp:707
#27 0xb56a6aa2 in nsLoadGroup::RemoveRequest (this=0xae1076d0, request=0xace74b40, ctxt=0x0, aStatus=0)
    at /home/alon/Dev/mozilla-central/netwerk/base/src/nsLoadGroup.cpp:680
#28 0xb5e558e0 in nsDocument::DoUnblockOnload (this=0xace40800) at /home/alon/Dev/mozilla-central/content/base/src/nsDocument.cpp:7341
#29 0xb5e3e488 in nsDocument::DispatchContentLoadedEvents (this=0xace40800)
    at /home/alon/Dev/mozilla-central/content/base/src/nsDocument.cpp:4195
#30 0xb5e5b03e in nsRunnableMethodImpl<void (nsDocument::*)(), true>::Run() () from dist/bin/libxul.so
#31 0xb7398b38 in nsThread::ProcessNextEvent (this=0xb3f06b00, mayWait=0, result=0xbfffebcc)
    at /home/alon/Dev/mozilla-central/xpcom/threads/nsThread.cpp:618
#32 0xb73062bf in NS_ProcessNextEvent_P (thread=0x0, mayWait=0) at /home/alon/Dev/m-c/xpcom/build/nsThreadUtils.cpp:250
#33 0xb719fed3 in mozilla::ipc::MessagePump::Run (this=0xb3fd6880, aDelegate=0xb3f20760)
    at /home/alon/Dev/mozilla-central/ipc/glue/MessagePump.cpp:110
#34 0xb740a6e5 in MessageLoop::RunInternal (this=0xb3f20760) at /home/alon/Dev/mozilla-central/ipc/chromium/src/base/message_loop.cc:218
#35 MessageLoop::RunHandler (this=0xb3f20760) at /home/alon/Dev/mozilla-central/ipc/chromium/src/base/message_loop.cc:202
#36 MessageLoop::Run (this=0xb3f20760) at /home/alon/Dev/mozilla-central/ipc/chromium/src/base/message_loop.cc:176
#37 0xb6fabe9c in nsBaseAppShell::Run (this=0xb247d470) at /home/alon/Dev/mozilla-central/widget/src/xpwidgets/nsBaseAppShell.cpp:189
#38 0xb6c183b0 in nsAppStartup::Run (this=0xb1a05550) at /home/alon/Dev/mozilla-central/toolkit/components/startup/nsAppStartup.cpp:224
#39 0xb564034b in XRE_main (argc=3, argv=0xbffff3a4, aAppData=0xb3f10380) at /home/alon/Dev/mozilla-central/toolkit/xre/nsAppRunner.cpp:3698
#40 0x0804971c in main (argc=3, argv=0xbffff3a4) at /home/alon/Dev/mozilla-central/browser/app/nsBrowserApp.cpp:159
(gdb)
Comment 1 Alon Zakai (:azakai) 2011-05-13 10:10:39 PDT
Calling code was added here

http://hg.mozilla.org/mozilla-central/rev/50de57368f92
Comment 2 Matt Woodrow (:mattwoodrow) 2011-05-13 14:56:41 PDT
Interesting, I didn't realize that this could be called from something other than JS.

I'm not sure what else we can really do here, other than null check and not mark the allocation in that case.
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2011-05-13 20:35:40 PDT
Anyone can clone DOM nodes from C++, yes.

The null-check is the right thing to do here; if your cloner is not JS you don't want to count the memory as GCable anyway.
Comment 4 Johnathan Nightingale [:johnath] 2011-05-16 14:37:28 PDT
Not tracking, but would like to approve a baked nullcheck patch
Comment 5 chris hofmann 2011-05-16 14:38:54 PDT
we should get a socorro signature for this crash, then match up against the top crash list to determine how many users might be hitting this.
Comment 6 Matt Woodrow (:mattwoodrow) 2011-05-16 14:45:01 PDT
Created attachment 532743 [details] [diff] [review]
NULL check the current canvas context
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2011-05-18 22:06:50 PDT
Comment on attachment 532743 [details] [diff] [review]
NULL check the current canvas context

r=me
Comment 8 Alon Zakai (:azakai) 2011-05-20 10:16:29 PDT
Pushed: http://hg.mozilla.org/mozilla-central/rev/9259bf6c8a94
Comment 9 Johnny Stenback (:jst, jst@mozilla.com) 2011-05-24 15:01:52 PDT
This landed before the migration to 6, no need to track this.
Comment 10 Josh Matthews [:jdm] 2011-05-31 18:54:55 PDT
Since the time for FF5 is nearly upon us, I propose that we take this baked null-check patch instead of the proposed replacement in bug 658440. How do approvals work in this brave new rapid release world?
Comment 11 Boris Zbarsky [:bz] (still a bit busy) 2011-05-31 19:20:23 PDT
They work by you requesting approval on the patch, with justification for why it should be approved and a risk/benefit assessment.  Probably pointing to comment 10 plus the risk/benefit thing is enough.
Comment 12 Josh Matthews [:jdm] 2011-05-31 19:41:29 PDT
Comment on attachment 532743 [details] [diff] [review]
NULL check the current canvas context

This is a nice, safe patch that prevents a crash. jst indicated in comment 5 that he'd like to approve this.
Comment 13 Johnathan Nightingale [:johnath] 2011-06-01 07:17:30 PDT
Comment on attachment 532743 [details] [diff] [review]
NULL check the current canvas context

That was me, not Johnny, but it remains true!
Comment 14 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2011-06-01 11:55:31 PDT
http://hg.mozilla.org/releases/mozilla-beta/rev/73efe358b1d7
Comment 15 Josh Matthews [:jdm] 2011-06-05 08:18:10 PDT
Just making this more visible for any followup crash stats.

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