Bug 15483 – REGRESSION: fast/images/svg-background-crash-on-refresh.html hangs

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Oliver Hunt]

fast/images/svg-background-crash-on-refresh.html causes a hang (not necessarily immediately) in run-webkit-tests

[Eric Seidel (no email)]

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.

[Eric Seidel (no email)]

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?

[Maciej Stachowiak]

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

[Maciej Stachowiak]

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.

[Maciej Stachowiak]

Created attachment 16677 [details]
second sample

[Maciej Stachowiak]

The result is the same running under GuardMalloc - hang with CG waiting for a spin lock in a later test.

[Eric Seidel (no email)]

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

[Eric Seidel (no email)]

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)

[Eric Seidel (no email)]

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.

[Nikolas Zimmermann]

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.

[Nikolas Zimmermann]

Landed in r87787.