testcase asserts in a debug build. _moz_cairo_set_scaled_font is called with scaled_font = NULL then the line cr->status = scaled_font->status; goes boom. Call stack: _moz_cairo_set_scaled_font gfxWindowsFont::SetupCairoFont gfxFont::Draw gfxWindowsFont::Draw gfxTextRun::DrawGlyphs gfxTextRun::Draw nsSVGGlyphFrame::LoopCharacters ... Prior to this gfxWindowsFont::MakeCairoScaledFont is called with mAdjustedSize = 0 so the call to cairo_scaled_font_create returns NULL

The patch in bug 390476 fixes this crash.

After bug 390476 was fixed it hangs instead.

The hang is a deadlock on _cairo_scaled_font_map_mutex: _moz_cairo_scaled_font_create() acquires the lock and calls font_face->backend->scaled_font_create(). We end up in _win32_scaled_font_create() which fails setup the font so it calls _moz_cairo_scaled_font_destroy() which tries to acquire the same mutex and hangs.

FWIW, this fixes it. Or we could make the mutex recursive, if we have support for that type of mutex.

Uhm, I'm pretty sure we set CAIRO_NO_MUTEX... though now that I look at it, that still creates some variables to use as faux mutexes. Ugh.

Right, it's their "poor man's mutex" that deadlocks on a recursive call from the same thread. They really should provide a true NOP alternative for applications that does all Cairo calls from the same thread. Saves us a few assignments too ;-)

Comment on attachment 278547 [details] [diff] [review] patch 2 Can you add a copy of this patch into the gfx/cairo dir, and add a note into the README about what it's for?

Actually, hold on -- I don't think that function should be calling scaled_font_destroy at all.

Comment on attachment 278547 [details] [diff] [review] patch 2 Yeah, the cairo_scaled_font_destroy (&f->base); should just be a free(f);

Good catch. I think we also need a _cairo_scaled_font_fini in case _cairo_scaled_font_init succeeded. I've included the patch in gfx/cairo/ and added an entry in README.

So do we still need the really-noop mutexes mutex code?

1. it saves a few unnecessary integer read/write ops 2. more importantly, it avoids deadlocking in case there is (or someone introduces) another recursive path to the same mutex

In theory the mutexes aren't supposed to be recursive; I think a recursive acquisition is supposed to be an error condition. That's the case for the linux mutexes at least, but not for the win32 ones (EnterCriticalSection allows for recursive acquisition). Though the poor man's mutex isn't really doing anyone any favours anyway...

mozilla/gfx/cairo/cairo/src/cairo-mutex-type-private.h 1.7 mozilla/gfx/cairo/cairo/src/cairo-win32-font.c 1.34 mozilla/gfx/cairo/README 1.62 mozilla/gfx/cairo/scaled-font-create-deadlock.patch 1.1 -> FIXED

Crash test: https://hg.mozilla.org/integration/mozilla-inbound/rev/38d9fbccbd63