Bug 15483 - REGRESSION: fast/images/svg-background-crash-on-refresh.html hangs
Summary: REGRESSION: fast/images/svg-background-crash-on-refresh.html hangs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 523.x (Safari 3)
Hardware: Macintosh All
: P1 Critical
Assignee: Nikolas Zimmermann
URL:
Keywords: Regression
Depends on: 15373
Blocks:
  Show dependency treegraph
 
Reported: 2007-10-12 17:38 PDT by Oliver Hunt
Modified: 2011-06-01 02:13 PDT (History)
2 users (show)

See Also:


Attachments
a sample of the hang (10.49 KB, text/plain)
2007-10-15 02:15 PDT, Maciej Stachowiak
no flags Details
second sample (2.37 KB, text/plain)
2007-10-15 02:15 PDT, Maciej Stachowiak
no flags Details
Patch (59.00 KB, patch)
2011-06-01 01:42 PDT, Nikolas Zimmermann
kling: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2007-10-12 17:38:00 PDT
fast/images/svg-background-crash-on-refresh.html causes a hang (not necessarily immediately) in run-webkit-tests
Comment 1 Eric Seidel (no email) 2007-10-14 22:42:02 PDT
This won't reproduce for me either. :(  At least not running run-webkit-tests fast or run-webkit-tests fast/images or running the test by itself using --guard-malloc.  When this test was originally written it was to catch problems during cache clearing (when the actual SVGImage's SVGDocument is destroyed), but those were fixed by a fix to ContainerNode::removeAllChildren().  I believe you there could be a problem with this test still, but I'm not able to reproduce it.
Comment 2 Eric Seidel (no email) 2007-10-15 01:03:40 PDT
I don't see this in either mac-leopard/Skipped or mac-tiger/Skipped.  It also doesn't seem to have a -disabled suffix.   As far as I can tell this test is being run and working for me every time.  Can anyone confirm this failure?
Comment 3 Maciej Stachowiak 2007-10-15 01:56:52 PDT
If I re-enable the test, here's the minimum command it takes for me to get the hang:

run-webkit-tests fast/images/svg-background-crash-on-refresh.html fast/inline/continuation-outlines-with-layers.html

Comment 4 Maciej Stachowiak 2007-10-15 02:15:25 PDT
Created attachment 16676 [details]
a sample of the hang

I suspect the SVG background has resulted in some sort of memory trashing, thus the stuck spinlock.
Comment 5 Maciej Stachowiak 2007-10-15 02:15:53 PDT
Created attachment 16677 [details]
second sample
Comment 6 Maciej Stachowiak 2007-10-15 02:45:42 PDT
The result is the same running under GuardMalloc - hang with CG waiting for a spin lock in a later test.
Comment 7 Eric Seidel (no email) 2007-10-15 22:07:51 PDT
FWIW run-webkit-tests fast/images/svg-background-crash-on-refresh.html fast/inline/continuation-outlines-with-layers.html succeeds for me.
Comment 8 Eric Seidel (no email) 2007-10-15 22:47:30 PDT
Ok, one possible problem is that the code in:
void SVGResourceMasker::applyMask(GraphicsContext* context, const FloatRect& boundingBox)

may cause the backing store for the CGImage (used as a mask) to be destroyed before CG actually uses it.  The ImageBuffer will destroy the backing store when it goes out of scope, however CG might not have made a copy of the bytes yet (making an image from a CGContext is COW) and thus it might be reading off into oblivion when it actually applies the mask.  How that would cause this hang?  no clue.

Also, we free the bytes behind the CGBitmapImageContext before we release the actual context and image, again, I'm not sure this is a "real problem" but we do.  Her is a little patch-chen which would fix that part:

Index: platform/graphics/cg/ImageBufferCG.cpp
===================================================================
--- platform/graphics/cg/ImageBufferCG.cpp      (revision 26601)
+++ platform/graphics/cg/ImageBufferCG.cpp      (working copy)
@@ -71,8 +71,10 @@
 
 ImageBuffer::~ImageBuffer()
 {
+    CGImageRelease(m_cgImage);
+    m_context = 0;
+    // Let go of our handles to the CGBitmapContext before we blow away its backing store
     fastFree(m_data);
-    CGImageRelease(m_cgImage);
 }
 
 GraphicsContext* ImageBuffer::context() const

Again, I can't reproduce this, so I'm not much use to try these fixes.  Someone could try hacking applyMask to leak the ImageBuffer and see if that fixed the problem.  (If it does we'll have to come up with a more elegant solution to make CG or GraphicsContext own the bytes)
Comment 9 Eric Seidel (no email) 2007-11-28 12:20:06 PST
This is likely a dup of bug 15373.  Since "hangs" in DRT generally are caused by too many printfs filling the stderr buffer that perl has, until the write() call just blocks in DRT.
Comment 10 Nikolas Zimmermann 2011-06-01 01:42:21 PDT
Created attachment 95565 [details]
Patch

Convert the test to use dumpAsText(), and make it actually refresh! If a crash happened, it happened somewhen after executing this test, instead of while executing this test. This is probably why it was flakey.
Comment 11 Nikolas Zimmermann 2011-06-01 02:13:09 PDT
Landed in r87787.