22444 2008-11-23 21:12:10 -0800 Bad Default Constructor for Cairo FontPlatformData Causes Infinite Loop in FontCache.cpp 2021-04-12 14:20:33 -0700 1 1 1 Unclassified WebKit WebKit Misc. 528+ (Nightly build) Mac Windows XP RESOLVED FIXED P2 Normal --- 1 bfulgham webkit-unassigned ap bolsinga oldest_to_newest 99869 0 bfulgham 2008-11-23 21:12:10 -0800 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. 99870 1 bfulgham 2008-11-23 21:18:17 -0800 Further testing shows that the two 99873 2 bfulgham 2008-11-23 21:29:47 -0800 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. 99876 3 bfulgham 2008-11-23 21:34:00 -0800 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. 99877 4 25411 bfulgham 2008-11-23 21:39:30 -0800 Created attachment 25411 Patch to revert static objects from DEFINE_STATIC_LOCAL 99885 5 ap 2008-11-23 23:11:52 -0800 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. 99973 6 bfulgham 2008-11-24 09:18:23 -0800 Compiler information: Visual Studio 2005 (8.0.50727.762, SP.050727-7600) 99980 7 bfulgham 2008-11-24 09:53:19 -0800 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. 99986 8 bfulgham 2008-11-24 10:34:20 -0800 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. 99990 9 25434 bfulgham 2008-11-24 10:55:00 -0800 Created attachment 25434 Correct default constructor for Cairo FontPlatformData. 99991 10 25435 bfulgham 2008-11-24 11:00:26 -0800 Created attachment 25435 Correct default constructor for Cairo FontPlatformData. Really! I mean it this time. 99994 11 25435 ap 2008-11-24 11:09:02 -0800 Comment on attachment 25435 Correct default constructor for Cairo FontPlatformData. r=me 99995 12 25437 bfulgham 2008-11-24 11:18:14 -0800 Created attachment 25437 Correcte default constructor for Cairo FontPlatformData. Corrected extra blank line. 99996 13 25437 ap 2008-11-24 11:19:27 -0800 Comment on attachment 25437 Correcte default constructor for Cairo FontPlatformData. r=me 99997 14 ap 2008-11-24 11:22:16 -0800 Committed revision 38713. 25411 2008-11-23 21:39:30 -0800 2008-11-24 10:55:00 -0800 Patch to revert static objects from DEFINE_STATIC_LOCAL font.patch text/plain 992 bfulgham YmZ1bGdoYW1AYmZ1bGdoYW0gfi9XZWJLaXQKJCBzdm4gZGlmZiBXZWJDb3JlL3BsYXRmb3JtL2dy YXBoaWNzL0ZvbnRDYWNoZS5jcHAKSW5kZXg6IFdlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvRm9u dENhY2hlLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL0ZvbnRD YWNoZS5jcHAgICAgIChyZXZpc2lvbiAzODY5NCkKKysrIFdlYkNvcmUvcGxhdGZvcm0vZ3JhcGhp Y3MvRm9udENhY2hlLmNwcCAgICAgKHdvcmtpbmcgY29weSkKQEAgLTEwNSw3ICsxMDUsNyBAQAog ICAgIHN0YXRpYyBjb25zdCBib29sIGVtcHR5VmFsdWVJc1plcm8gPSB0cnVlOwogICAgIHN0YXRp YyBjb25zdCBGb250UGxhdGZvcm1EYXRhQ2FjaGVLZXkmIGVtcHR5VmFsdWUoKQogICAgIHsKLSAg ICAgICAgREVGSU5FX1NUQVRJQ19MT0NBTChGb250UGxhdGZvcm1EYXRhQ2FjaGVLZXksIGtleSwg KG51bGxBdG9tKSk7CisgICAgICAgIHN0YXRpYyBGb250UGxhdGZvcm1EYXRhQ2FjaGVLZXkga2V5 KG51bGxBdG9tKTsKICAgICAgICAgcmV0dXJuIGtleTsKICAgICB9CiAgICAgc3RhdGljIHZvaWQg Y29uc3RydWN0RGVsZXRlZFZhbHVlKEZvbnRQbGF0Zm9ybURhdGFDYWNoZUtleSYgc2xvdCkKQEAg LTIwNiw3ICsyMDYsNyBAQAogICAgIHN0YXRpYyBjb25zdCBib29sIG5lZWRzRGVzdHJ1Y3Rpb24g PSB0cnVlOwogICAgIHN0YXRpYyBjb25zdCBGb250UGxhdGZvcm1EYXRhJiBlbXB0eVZhbHVlKCkK ICAgICB7Ci0gICAgICAgIERFRklORV9TVEFUSUNfTE9DQUwoRm9udFBsYXRmb3JtRGF0YSwga2V5 LCAoKSk7CisgICAgICAgIHN0YXRpYyBGb250UGxhdGZvcm1EYXRhIGtleTsKICAgICAgICAgcmV0 dXJuIGtleTsKICAgICB9CiAgICAgc3RhdGljIHZvaWQgY29uc3RydWN0RGVsZXRlZFZhbHVlKEZv bnRQbGF0Zm9ybURhdGEmIHNsb3QpCgo= 25434 2008-11-24 10:55:00 -0800 2008-11-24 11:00:36 -0800 Correct default constructor for Cairo FontPlatformData. fontfix.patch text/plain 1214 bfulgham SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiAzNTEwMikKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMDgtMDctMTAgIEJyZW50IEZ1bGdoYW0gIDxiZnVsZ2hhbUBnbWFp bC5jb20+CisKKyAgICAgICAgQ29ycmVjdCBhIGJ1aWxkIHJlZ3Jlc3Npb24gZHVlIHRvIGFuIHVu aW5pdGlhbGl6ZWQgdmFyaWFibGUuCisKKyAgICAgICAgPGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0xOTk3Nj4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBXQVJOSU5HOiBOTyBURVNUIENBU0VTIEFEREVEIE9SIENIQU5HRUQK KworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL2NhaXJvL0ZvbnRDYWlyby5jcHA6CisgICAg ICAgIChXZWJDb3JlOjpGb250OjpkcmF3R2x5cGhzKToKKwogMjAwOC0wNy0xMCAgS2V2aW4gTWND dWxsb3VnaCAgPGttY2N1bGxvdWdoQGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBE YXJpbi4KSW5kZXg6IFdlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2Fpcm8vRm9udENhaXJvLmNw cAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2NhaXJvL0ZvbnRDYWly by5jcHAJKHJldmlzaW9uIDM1MDk5KQorKysgV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jYWly by9Gb250Q2Fpcm8uY3BwCSh3b3JraW5nIGNvcHkpCkBAIC01NSw3ICs1NSw3IEBAIHZvaWQgRm9u dDo6ZHJhd0dseXBocyhHcmFwaGljc0NvbnRleHQqIGMKIAogICAgIC8vIFRleHQgc2hhZG93LCBp bnNwaXJlZCBieSBGb250TWFjCiAgICAgSW50U2l6ZSBzaGFkb3dTaXplOwotICAgIGludCBzaGFk b3dCbHVyOworICAgIGludCBzaGFkb3dCbHVyID0gMDsKICAgICBDb2xvciBzaGFkb3dDb2xvcjsK ICAgICBib29sIGhhc1NoYWRvdyA9IGNvbnRleHQtPnRleHREcmF3aW5nTW9kZSgpID09IGNUZXh0 RmlsbCAmJgogICAgICAgICBjb250ZXh0LT5nZXRTaGFkb3coc2hhZG93U2l6ZSwgc2hhZG93Qmx1 ciwgc2hhZG93Q29sb3IpOwo= 25435 2008-11-24 11:00:26 -0800 2008-11-24 11:18:14 -0800 Correct default constructor for Cairo FontPlatformData. fontdata.patch text/plain 1248 bfulgham SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiAzODcxMikKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTggQEAKKzIwMDgtMTEtMjQgIEJyZW50IEZ1bGdoYW0gIDxiZnVsZ2hhbUBnbWFp bC5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIyNDQ0CisKKyAgICAgICAg Q29ycmVjdCBiYWQgZGVmYXVsdCBjb25zdHJ1Y3RvciBmb3IgQ2Fpcm8gRm9udFBsYXRmb3JtRGF0 YSBzbyB0aGF0CisgICAgICAgIHRoZSBtX3NjYWxlZEZvbnQgbWVtYmVyIGlzIGluaXRpYWxpemVk LiAgUHJldmlvdXNseSB0aGlzIHdvdWxkIGJlCisgICAgICAgIHNldCB0byAweGJhYWRmMDBkLCB3 aGljaCB3b3VsZCByZXN1bHQgaW4gaW5maW5pdGUgbG9vcHMgd2hpbGUKKyAgICAgICAgYXR0ZW1w dGluZyB0byBkZWFsIHdpdGggdGhlIGZvbnQgY2FjaGUuCisKKworICAgICAgICAqIHBsYXRmb3Jt L2dyYXBoaWNzL3dpbi9Gb250UGxhdGZvcm1EYXRhLmg6CisgICAgICAgIChXZWJDb3JlOjpGb250 UGxhdGZvcm1EYXRhOjpGb250UGxhdGZvcm1EYXRhKTogSW5pdGlhbGl6ZSBtX3NjYWxlZEZvbnQg dG8gMAorCiAyMDA4LTExLTI0ICBBbGV4ZXkgUHJvc2t1cnlha292ICA8YXBAd2Via2l0Lm9yZz4K IAogICAgICAgICBSZXZpZXdlZCBieSBEYXJpbiBBZGxlci4KSW5kZXg6IFdlYkNvcmUvcGxhdGZv cm0vZ3JhcGhpY3Mvd2luL0ZvbnRQbGF0Zm9ybURhdGEuaAo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3Jl L3BsYXRmb3JtL2dyYXBoaWNzL3dpbi9Gb250UGxhdGZvcm1EYXRhLmgJKHJldmlzaW9uIDM4Njk0 KQorKysgV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy93aW4vRm9udFBsYXRmb3JtRGF0YS5oCSh3 b3JraW5nIGNvcHkpCkBAIC00NSw2ICs0NSw3IEBAIHB1YmxpYzoKICAgICBGb250UGxhdGZvcm1E YXRhKCkKICNpZiBQTEFURk9STShDQUlSTykKICAgICAgICAgOiBtX2ZvbnRGYWNlKDApCisgICAg ICAgICwgbV9zY2FsZWRGb250KDApCiAgICAgICAgICwKICNlbHNlCiAgICAgICAgIDoK 25437 2008-11-24 11:18:14 -0800 2008-11-24 11:19:27 -0800 Correcte default constructor for Cairo FontPlatformData. fontdata.patch text/plain 1246 bfulgham SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiAzODcxMikKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTcgQEAKKzIwMDgtMTEtMjQgIEJyZW50IEZ1bGdoYW0gIDxiZnVsZ2hhbUBnbWFp bC5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIyNDQ0CisKKyAgICAgICAg Q29ycmVjdCBiYWQgZGVmYXVsdCBjb25zdHJ1Y3RvciBmb3IgQ2Fpcm8gRm9udFBsYXRmb3JtRGF0 YSBzbyB0aGF0CisgICAgICAgIHRoZSBtX3NjYWxlZEZvbnQgbWVtYmVyIGlzIGluaXRpYWxpemVk LiAgUHJldmlvdXNseSB0aGlzIHdvdWxkIGJlCisgICAgICAgIHNldCB0byAweGJhYWRmMDBkLCB3 aGljaCB3b3VsZCByZXN1bHQgaW4gaW5maW5pdGUgbG9vcHMgd2hpbGUKKyAgICAgICAgYXR0ZW1w dGluZyB0byBkZWFsIHdpdGggdGhlIGZvbnQgY2FjaGUuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9n cmFwaGljcy93aW4vRm9udFBsYXRmb3JtRGF0YS5oOgorICAgICAgICAoV2ViQ29yZTo6Rm9udFBs YXRmb3JtRGF0YTo6Rm9udFBsYXRmb3JtRGF0YSk6IEluaXRpYWxpemUgbV9zY2FsZWRGb250IHRv IDAKKwogMjAwOC0xMS0yNCAgQWxleGV5IFByb3NrdXJ5YWtvdiAgPGFwQHdlYmtpdC5vcmc+CiAK ICAgICAgICAgUmV2aWV3ZWQgYnkgRGFyaW4gQWRsZXIuCkluZGV4OiBXZWJDb3JlL3BsYXRmb3Jt L2dyYXBoaWNzL3dpbi9Gb250UGxhdGZvcm1EYXRhLmgKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9w bGF0Zm9ybS9ncmFwaGljcy93aW4vRm9udFBsYXRmb3JtRGF0YS5oCShyZXZpc2lvbiAzODY5NCkK KysrIFdlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvd2luL0ZvbnRQbGF0Zm9ybURhdGEuaAkod29y a2luZyBjb3B5KQpAQCAtNDUsNiArNDUsNyBAQCBwdWJsaWM6CiAgICAgRm9udFBsYXRmb3JtRGF0 YSgpCiAjaWYgUExBVEZPUk0oQ0FJUk8pCiAgICAgICAgIDogbV9mb250RmFjZSgwKQorICAgICAg ICAsIG1fc2NhbGVkRm9udCgwKQogICAgICAgICAsCiAjZWxzZQogICAgICAgICA6Cg==