1270878 - Assertion failure: int32_t(mRefCnt) > 0 (dup release), at gfx/thebes/gfxDWriteFontList.cpp:1730 on Windows 8 debug

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Tim Guan-tin Chien [:timdream] (please needinfo)]

This bug prevents bug 1231701 from being fixed, and I figured to reduce the noise I should create a new bug instead. The stack is as follows and it happens only on Win8 (win64) debug builds, after turning on MOZ_BUNDLED_FONTS. The flag has no effect on opt build, for no obvious reasons.

I was also being pointed out that MOZ_BUNDLED_FONTS was not tested at all since B2G builds (& mulet) got tier-3'd. It's pretty sad that we would need to fix this after-the-fact.


05:14:31     INFO -  Assertion failure: int32_t(mRefCnt) > 0 (dup release), at c:/builds/moz2_slave/try-w64-d-00000000000000000000/build/src/gfx/thebes/gfxDWriteFontList.cpp:1730
05:14:31     INFO -  #01: dwrite.dll + 0x61df9
05:14:31     INFO -  #02: dwrite.dll + 0x3954a
05:14:31     INFO -  #03: dwrite.dll + 0x394b9
05:14:31     INFO -  #04: dwrite.dll + 0x39329
05:14:31     INFO -  #05: dwrite.dll + 0x3925d
05:14:31     INFO -  #06: dwrite.dll + 0x39166
05:14:31     INFO -  #07: dwrite.dll + 0x5f323
05:14:31     INFO -  #08: dwrite.dll + 0x87682
05:14:47     INFO -  #09: gfxDWriteFontList::CreateBundledFontsCollection(IDWriteFactory *) [gfx/thebes/gfxDWriteFontList.cpp:1854]
05:14:47     INFO -  #10: gfxDWriteFontList::InitFontList() [gfx/thebes/gfxDWriteFontList.cpp:917]
05:14:47     INFO -  #11: gfxWindowsPlatform::CreatePlatformFontList() [gfx/thebes/gfxWindowsPlatform.cpp:576]
05:14:47     INFO -  #12: gfxPlatformFontList::Init() [gfx/thebes/gfxPlatformFontList.h:103]
05:14:47     INFO -  #13: gfxPlatform::Init() [gfx/thebes/gfxPlatform.cpp:686]
05:14:47     INFO -  #14: gfxPlatform::GetPlatform() [gfx/thebes/gfxPlatform.cpp:510]
05:14:47     INFO -  #15: mozilla::widget::GfxInfo::GetD2DEnabled(bool *) [widget/windows/GfxInfo.cpp:49]

Comment 1

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I can reproduce this locally on the Win8 machine in the office, from the build downloaded from try. I've set the machine up to create a local build so I could debug with it on next week.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: Factory method that creates a BundledFontEnumerator needs to ensure it is AddRef'd before being returned to the caller

This has always been broken, AFAICS, since it first landed in bug 998844. To comply with Microsoft COM expectations, we need to increment the object's refcount before returning it from a factory method. I guess we never ran Mulet debug tests, or whatever...

Comment 3

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Jonathan, thanks for the quick fix! I kind of reached the same patch as this one yesterday, but the bundled font is still not not showing up -- appearently the code did not work even if we stop the crash. Do you have a local build to verify that? I will be doing the same too.

Comment 4

[Tim Guan-tin Chien [:timdream] (please needinfo)]

With breakpoint and stepping in VS2015, I found that |fontEntry->HasCharacter(aCh)| returns false for aCh = 0x0001f4a9, the Pile of poo character.

https://dxr.mozilla.org/mozilla-central/rev/043082cb7bd8490c60815f67fbd1f33323ad7663/gfx/thebes/gfxPlatformFontList.cpp#597

Further inspection shows `blockIndex >= mBlocks.Length()` evaluates to false (blockIndex = 0x1f4 and mBlocks.Length() = 0x100), which suggests we are not reading the font CMAP correctly.

https://dxr.mozilla.org/mozilla-central/rev/043082cb7bd8490c60815f67fbd1f33323ad7663/gfx/thebes/gfxFontUtils.h#74-85

This does not looks right because we are supposed to parse the CMAP in our own code on every platform, no?

Comment 5

[Tim Guan-tin Chien [:timdream] (please needinfo)]

The above comments is now bug 1271536.

Comment 6

[Bas Schouten (:bas.schouten)]

Comment on attachment 8750389 [details] [diff] [review]
Factory method that creates a BundledFontEnumerator needs to ensure it is AddRef'd before being returned to the caller

Review of attachment 8750389 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch.

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Thank you for your quick help in bug 1271536!

Comment 8

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5ec5cf288a24c02ff7e0d9c13f0e6ccec3adac2d
Bug 1270878 - Factory method that creates a BundledFontEnumerator needs to ensure it is AddRef'd before being returned to the caller. r=bas

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5ec5cf288a24