Bug 22444 – Bad Default Constructor for Cairo FontPlatformData Causes Infinite Loop in FontCache.cpp

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Brent Fulgham]

I tracked down the cause of a deadlock that began appearing after some of the changes starting at @r35364 through current.

Behavior:  Launching the WinLauncher executable executes to the point that the main frame is displayed, but the test rendering page is never displayed.  Breaking in the debugger repeatedly revealed that the program was locked attempting to set a missing font entry in the font cache:

[FontCache.cpp: 174]        gFontPlatformDataCache->set(key, result);

The debugger was actually stuck in the hash table implementation (JSC::hashable.h, static bool isDeletedBucket).

By working from an existing good source tree, I found that the problem occurs as soon as the new DEFINE_STATIC_LOCAL calls were introduced into the FontCache object.

[Brent Fulgham]

Further testing shows that the two 

[Brent Fulgham]

Further testing shows that the problem is with the two static emptyValue methods:

FontDataCacheKeyTraits::emptyValue
FontPlatformDataCacheKeyTraits::emptyValue

If either (or both) of these methods makes use of the DEFINE_STATIC_LOCAL macro the WebView will deadlock during rendering of the test page.  The other DEFINE_STATIC_LOCAL macro uses do not seem to create any problems.

[Brent Fulgham]

Test system:

Windows XP, 3.5 GB RAM.  2.9 GHZ Pentium D CPU.

I don't know if this bug is exhibited due to the lack of true dual cores on this test system, but I suspect the issue is related in some fashion to the change from static objects existing for the full life of the program, and the newer 'on-demand' style of initialization.

[Brent Fulgham]

Created attachment 25411 [details]
Patch to revert static objects from DEFINE_STATIC_LOCAL

[Alexey Proskuryakov]

An infinite loop in HashTable is usually caused by trashed memory. I wonder if MSVC could also have a bug WRT lifetime of static references.

[Brent Fulgham]

Compiler information:
Visual Studio 2005 (8.0.50727.762, SP.050727-7600)

[Brent Fulgham]

The specific problem seems to be that the HashTable implementation 'add' method can't figure out how to handle the key that was passed into the routine.  If breakpoints are placed on each of the ways of breaking out of the "while (1)" loop in hashtable.h (around line 636-653), none are ever hit.

The key that is passed in appears to be fully-constructed.

It first calls "isEmptyOrDeleted", which calls the troublesome "emptyValue" method.  It appears to keep returning the same address for its object.  However, changing the declaration back to a stack-allocated object causes the routine to perform properly.  The return value is used in an equality operation, which indicates that the two objects are not the same .

They are not the same because m_scaledFont is set to 0xbaadf00d in the 'emptyValue' object, while the passed in key is set to 0x00.

[Brent Fulgham]

Problem was traced to the FontPlatformData.h default constructor.  This did not initialize the Cairo-specific m_scaledFont member, resulting in it defaulting to 0xbaadf00d.  Previously, the stack-allocated static nature of the object triggered the object to be 0-initialized before use.

Patch to come.

[Brent Fulgham]

Created attachment 25434 [details]
Correct default constructor for Cairo FontPlatformData.

[Brent Fulgham]

Created attachment 25435 [details]
Correct default constructor for Cairo FontPlatformData.

Really!  I mean it this time.

[Alexey Proskuryakov]

Comment on attachment 25435 [details]
Correct default constructor for Cairo FontPlatformData.

r=me

[Brent Fulgham]

Created attachment 25437 [details]
Correcte default constructor for Cairo FontPlatformData.

Corrected extra blank line.

[Alexey Proskuryakov]

Comment on attachment 25437 [details]
Correcte default constructor for Cairo FontPlatformData.

r=me

[Alexey Proskuryakov]

Committed revision 38713.