30408 2009-10-15 14:49:04 -0700 ThreadSpecific instances or ThreadGlobalData instance is being leaked when the WebKit library is being reloaded. 2009-10-16 14:44:08 -0700 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) PC OS X 10.5 RESOLVED DUPLICATE 27980 P2 Normal --- 0 carol webkit-unassigned ap mrowe oldest_to_newest 154910 0 carol 2009-10-15 14:49:04 -0700 Some of WebKit including ThreadGlobalData implementation is written with the assumption that some singletons will be cleaned up by the OS when the process quits and therefore they can be leaked. Beside this not being an elegant solution, it does not cover all cases such as the case when WebKit is used as component or widget in a larger process and hence the library may be loaded and unloaded multiple times before the process quits. With each load and unload memory (and genrally resources) associated with these singletons is leaked. My goal is to chase down these singletons and ensure that they are destroyed when the library is unloaded. One strategy in doing this is to transform the pointers to global static objects. Another is to transform pointers into smart pointers such as OwnPtr. Patches to follow. 154967 1 41258 carol 2009-10-15 18:18:55 -0700 Created attachment 41258 Proposed Patch This addresses the singleton problem related to threadGlobalData(). 154971 2 41258 ap 2009-10-15 18:57:38 -0700 Comment on attachment 41258 Proposed Patch + Allocated threadGlobalData statically, not on heap such that it + will be destroyed when the library is unloaded. We don't allow static objects with destructors, because we want application shutdown to be fast. 155027 3 mrowe 2009-10-15 23:29:05 -0700 Many aspects of the WebKit code are written with the assumption that the library will not be unloaded prior to exit. 155174 4 41310 carol 2009-10-16 12:48:05 -0700 Created attachment 41310 Proposed Patch While fast exit is generally a desirable feature for any application the lack of an elegant cleanup has its drawbacks: 1. There are some devices where almost all applications run in the same process space, thus when an application quits, it's libraries are unloaded from memory, but the application is expected to do its own cleanup. 2. When testing for memory leaks, these never cleaned singletons and their dependencies pollute the list of detected leaks that tools like valgrind generate. Hence this patch tries to be a step towards the best of both worlds: 1. For those for whom a quick exit is the only goal, this patch provides nothing but consistency in the way singletons are defined. No overhead, no change. 2. For the other camp for whom an elegant cleanup is important (such as style purists, testers and developers for antiquated OSes) a relatively clean and easy method is provided to have the singletons cleaned up before the WebKit library is unloaded: define the DEFINE_STATIC_LOCAL macro on the compiler's command line like this: "-DDEFINE_STATIC_LOCAL(type, name, arguments)=type name arguments" 155255 5 41310 ap 2009-10-16 14:43:57 -0700 Comment on attachment 41310 Proposed Patch If you want to release memory on exit, you need to somehow set an order to that, ensuring that released objects don't get accessed from destructors of other objects. In particular, C++ makes no guarantee that "static type&" will outlive all references to it. ThreadLocalData in particular is very susceptible to this problem. This was discussed in much detail on webkit-dev mailing list, and this work is tracked by bug 27980 (I personally don't expect that to ever be finished, due to high complexity and relatively low benefit). I'm going to mark this as a duplicate of bug 27980. 155256 6 ap 2009-10-16 14:44:08 -0700 *** This bug has been marked as a duplicate of bug 27980 *** 41258 2009-10-15 18:18:55 -0700 2009-10-16 12:48:05 -0700 Proposed Patch patchFor30408-1.patch text/plain 2109 carol SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0OTY2NykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTkgQEAKKzIwMDktMTAtMTUgIENhcm9sIFN6YWJvICA8Y2Fyb2wuc3phYm9Abm9r aWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFJlc291cmNlcyBhc3NvY2lhdGVkIHdpdGggc29tZSBzaW5nbGV0b25zIGFyZSBub3QgZnJlZWQg d2hlbiB0aGUKKyAgICAgICAgV2ViS2l0IGxpYnJhcnkgaXMgdW5sb2FkZWQuCisgICAgICAgIGh0 dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zMDQwOAorCisgICAgICAgIE5v IG5ldyB0ZXN0cyBmb3IgdGhpcyBhcyB0aGUgY2hhbmdlIGRvZXMgbm90IGFmZmVjdCBXZWJLaXQK KyAgICAgICAgZnVuY3Rpb25hbGl0eSwgYnV0IG9ubHkgbWVtb3J5IHVzYWdlLgorCisgICAgICAg ICogcGxhdGZvcm0vVGhyZWFkR2xvYmFsRGF0YS5jcHA6CisgICAgICAgIChXZWJDb3JlOjp0aHJl YWRHbG9iYWxEYXRhKToKKyAgICAgICAgQWxsb2NhdGVkIHRocmVhZEdsb2JhbERhdGEgc3RhdGlj YWxseSwgbm90IG9uIGhlYXAgc3VjaCB0aGF0IGl0CisgICAgICAgIHdpbGwgYmUgZGVzdHJveWVk IHdoZW4gdGhlIGxpYnJhcnkgaXMgdW5sb2FkZWQuCisKIDIwMDktMTAtMTQgIEpvbiBIb25leWN1 dHQgIDxqaG9uZXljdXR0QGFwcGxlLmNvbT4KIAogICAgICAgICBBZGQgU1BJIHRvIGRldGVybWlu ZSB3aGV0aGVyIGEgbm9kZSBpcyBhIGhhbHRlZCBwbHVnLWluLgpJbmRleDogV2ViQ29yZS9wbGF0 Zm9ybS9UaHJlYWRHbG9iYWxEYXRhLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL3BsYXRmb3Jt L1RocmVhZEdsb2JhbERhdGEuY3BwCShyZXZpc2lvbiA0OTUwMikKKysrIFdlYkNvcmUvcGxhdGZv cm0vVGhyZWFkR2xvYmFsRGF0YS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTU0LDE2ICs1NCwxMSBA QCBUaHJlYWRHbG9iYWxEYXRhJiB0aHJlYWRHbG9iYWxEYXRhKCkKICAgICAvLyBXZSBuZWVkIHRv IGNoZWNrIGZvciBlLmcuIGRhdGFiYXNlIG9iamVjdHMgbWFuaXB1bGF0aW5nIHN0cmluZ3Mgb24g c2Vjb25kYXJ5IHRocmVhZHMuCiAjaWYgRU5BQkxFKFdPUktFUlMpCiAgICAgLy8gVGhyZWFkR2xv YmFsRGF0YSBpcyB1c2VkIG9uIG1haW4gdGhyZWFkIGJlZm9yZSBpdCBjb3VsZCBwb3NzaWJseSBi ZSB1c2VkIG9uIHNlY29uZGFyeSBvbmVzLCBzbyB0aGVyZSBpcyBubyBuZWVkIGZvciBzeW5jaHJv bml6YXRpb24gaGVyZS4KLSAgICBzdGF0aWMgVGhyZWFkU3BlY2lmaWM8VGhyZWFkR2xvYmFsRGF0 YT4qIHRocmVhZEdsb2JhbERhdGEgPSBuZXcgVGhyZWFkU3BlY2lmaWM8VGhyZWFkR2xvYmFsRGF0 YT47Ci0gICAgcmV0dXJuICoqdGhyZWFkR2xvYmFsRGF0YTsKKyAgICBzdGF0aWMgVGhyZWFkU3Bl Y2lmaWM8VGhyZWFkR2xvYmFsRGF0YT4gdGhyZWFkR2xvYmFsRGF0YTsKKyAgICByZXR1cm4gKnRo cmVhZEdsb2JhbERhdGE7CiAjZWxzZQotICAgIHN0YXRpYyBUaHJlYWRHbG9iYWxEYXRhKiBzdGF0 aWNEYXRhOwotICAgIGlmICghc3RhdGljRGF0YSkgewotICAgICAgICBzdGF0aWNEYXRhID0gc3Rh dGljX2Nhc3Q8VGhyZWFkR2xvYmFsRGF0YSo+KGZhc3RNYWxsb2Moc2l6ZW9mKFRocmVhZEdsb2Jh bERhdGEpKSk7Ci0gICAgICAgIC8vIFRocmVhZEdsb2JhbERhdGEgY29uc3RydWN0b3IgaW5kaXJl Y3RseSB1c2VzIHN0YXRpY0RhdGEsIHNvIHdlIG5lZWQgdG8gc2V0IHVwIHRoZSBtZW1vcnkgYmVm b3JlIGludm9raW5nIGl0LgotICAgICAgICBuZXcgKHN0YXRpY0RhdGEpIFRocmVhZEdsb2JhbERh dGE7Ci0gICAgfQotICAgIHJldHVybiAqc3RhdGljRGF0YTsKKyAgICBzdGF0aWMgVGhyZWFkR2xv YmFsRGF0YSBzdGF0aWNEYXRhOworICAgIHJldHVybiBzdGF0aWNEYXRhOwogI2VuZGlmCiB9CiAK 41310 2009-10-16 12:48:05 -0700 2009-10-16 14:43:57 -0700 Proposed Patch patchFor30408-2.patch text/plain 3473 carol SW5kZXg6IEphdmFTY3JpcHRDb3JlL3d0Zi9TdGRMaWJFeHRyYXMuaAo9PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBK YXZhU2NyaXB0Q29yZS93dGYvU3RkTGliRXh0cmFzLmgJKHJldmlzaW9uIDQ5NTAyKQorKysgSmF2 YVNjcmlwdENvcmUvd3RmL1N0ZExpYkV4dHJhcy5oCSh3b3JraW5nIGNvcHkpCkBAIC0zMiw2ICsz Miw3IEBACiAvLyBVc2UgdGhlc2UgdG8gZGVjbGFyZSBhbmQgZGVmaW5lIGEgc3RhdGljIGxvY2Fs IHZhcmlhYmxlIChzdGF0aWMgVDspIHNvIHRoYXQKIC8vICBpdCBpcyBsZWFrZWQgc28gdGhhdCBp dHMgZGVzdHJ1Y3RvcnMgYXJlIG5vdCBjYWxsZWQgYXQgZXhpdC4gVXNpbmcgdGhpcwogLy8gIG1h Y3JvIGFsc28gYWxsb3dzIHdvcmthcm91bmRzIGEgY29tcGlsZXIgYnVnIHByZXNlbnQgaW4gQXBw bGUncyB2ZXJzaW9uIG9mIEdDQyA0LjAuMS4KKyNpZm5kZWYgREVGSU5FX1NUQVRJQ19MT0NBTAog I2lmIENPTVBJTEVSKEdDQykgJiYgZGVmaW5lZChfX0FQUExFX0NDX18pICYmIF9fR05VQ19fID09 IDQgJiYgX19HTlVDX01JTk9SX18gPT0gMCAmJiBfX0dOVUNfUEFUQ0hMRVZFTF9fID09IDEKICNk ZWZpbmUgREVGSU5FX1NUQVRJQ19MT0NBTCh0eXBlLCBuYW1lLCBhcmd1bWVudHMpIFwKICAgICBz dGF0aWMgdHlwZSogbmFtZSMjUHRyID0gbmV3IHR5cGUgYXJndW1lbnRzOyBcCkBAIC00MCw3ICs0 MSw3IEBACiAjZGVmaW5lIERFRklORV9TVEFUSUNfTE9DQUwodHlwZSwgbmFtZSwgYXJndW1lbnRz KSBcCiAgICAgc3RhdGljIHR5cGUmIG5hbWUgPSAqbmV3IHR5cGUgYXJndW1lbnRzCiAjZW5kaWYK LQorI2VuZGlmCiAvLyBPQkpFQ1RfT0ZGU0VUT0Y6IExpa2UgdGhlIEMrKyBvZmZzZXRvZiBtYWNy bywgYnV0IHlvdSBjYW4gdXNlIGl0IHdpdGggY2xhc3Nlcy4KIC8vIFRoZSBtYWdpYyBudW1iZXIg MHg0MDAwIGlzIGluc2lnbmlmaWNhbnQuIFdlIHVzZSBpdCB0byBhdm9pZCB1c2luZyBOVUxMLCBz aW5jZQogLy8gTlVMTCBjYW4gY2F1c2UgY29tcGlsZXIgcHJvYmxlbXMsIGVzcGVjaWFsbHkgaW4g Y2FzZXMgb2YgbXVsdGlwbGUgaW5oZXJpdGFuY2UuCkluZGV4OiBXZWJDb3JlL0NoYW5nZUxvZwo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09Ci0tLSBXZWJDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gNDk2NjcpCisrKyBXZWJD b3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBACisyMDA5LTEwLTE1 ICBDYXJvbCBTemFibyAgPGNhcm9sLnN6YWJvQG5va2lhLmNvbT4KKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBSZXNvdXJjZXMgYXNzb2NpYXRlZCB3aXRo IHNvbWUgc2luZ2xldG9ucyBhcmUgbm90IGZyZWVkIHdoZW4gdGhlCisgICAgICAgIFdlYktpdCBs aWJyYXJ5IGlzIHVubG9hZGVkLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MzA0MDgKKworICAgICAgICBObyBuZXcgdGVzdHMgZm9yIHRoaXMgYXMgdGhl IGNoYW5nZSBkb2VzIG5vdCBhZmZlY3QgV2ViS2l0CisgICAgICAgIGZ1bmN0aW9uYWxpdHksIGJ1 dCBvbmx5IG1lbW9yeSB1c2FnZS4KKworICAgICAgICAqIHBsYXRmb3JtL1RocmVhZEdsb2JhbERh dGEuY3BwOgorICAgICAgICAoV2ViQ29yZTo6dGhyZWFkR2xvYmFsRGF0YSk6CisgICAgICAgIEFs bG9jYXRlZCB0aHJlYWRHbG9iYWxEYXRhIHN0YXRpY2FsbHksIG5vdCBvbiBoZWFwIHN1Y2ggdGhh dCBpdAorICAgICAgICB3aWxsIGJlIGRlc3Ryb3llZCB3aGVuIHRoZSBsaWJyYXJ5IGlzIHVubG9h ZGVkLgorCiAyMDA5LTEwLTE0ICBKb24gSG9uZXljdXR0ICA8amhvbmV5Y3V0dEBhcHBsZS5jb20+ CiAKICAgICAgICAgQWRkIFNQSSB0byBkZXRlcm1pbmUgd2hldGhlciBhIG5vZGUgaXMgYSBoYWx0 ZWQgcGx1Zy1pbi4KSW5kZXg6IFdlYkNvcmUvcGxhdGZvcm0vVGhyZWFkR2xvYmFsRGF0YS5jcHAK PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PQotLS0gV2ViQ29yZS9wbGF0Zm9ybS9UaHJlYWRHbG9iYWxEYXRhLmNwcAkocmV2 aXNpb24gNDk1MDIpCisrKyBXZWJDb3JlL3BsYXRmb3JtL1RocmVhZEdsb2JhbERhdGEuY3BwCSh3 b3JraW5nIGNvcHkpCkBAIC0zMCw2ICszMCw3IEBACiAjaW5jbHVkZSAiRXZlbnROYW1lcy5oIgog I2luY2x1ZGUgIlN0cmluZ0ltcGwuaCIKICNpbmNsdWRlICJUaHJlYWRUaW1lcnMuaCIKKyNpbmNs dWRlIDx3dGYvU3RkTGliRXh0cmFzLmg+CiAjaW5jbHVkZSA8d3RmL1VudXNlZFBhcmFtLmg+CiAK ICNpZiBVU0UoSUNVX1VOSUNPREUpIHx8IFVTRShHTElCX0lDVV9VTklDT0RFX0hZQlJJRCkKQEAg LTU0LDE2ICs1NSwxMSBAQCBUaHJlYWRHbG9iYWxEYXRhJiB0aHJlYWRHbG9iYWxEYXRhKCkKICAg ICAvLyBXZSBuZWVkIHRvIGNoZWNrIGZvciBlLmcuIGRhdGFiYXNlIG9iamVjdHMgbWFuaXB1bGF0 aW5nIHN0cmluZ3Mgb24gc2Vjb25kYXJ5IHRocmVhZHMuCiAjaWYgRU5BQkxFKFdPUktFUlMpCiAg ICAgLy8gVGhyZWFkR2xvYmFsRGF0YSBpcyB1c2VkIG9uIG1haW4gdGhyZWFkIGJlZm9yZSBpdCBj b3VsZCBwb3NzaWJseSBiZSB1c2VkIG9uIHNlY29uZGFyeSBvbmVzLCBzbyB0aGVyZSBpcyBubyBu ZWVkIGZvciBzeW5jaHJvbml6YXRpb24gaGVyZS4KLSAgICBzdGF0aWMgVGhyZWFkU3BlY2lmaWM8 VGhyZWFkR2xvYmFsRGF0YT4qIHRocmVhZEdsb2JhbERhdGEgPSBuZXcgVGhyZWFkU3BlY2lmaWM8 VGhyZWFkR2xvYmFsRGF0YT47Ci0gICAgcmV0dXJuICoqdGhyZWFkR2xvYmFsRGF0YTsKKyAgICBE RUZJTkVfU1RBVElDX0xPQ0FMKFRocmVhZFNwZWNpZmljPFRocmVhZEdsb2JhbERhdGE+LCB0aHJl YWRHbG9iYWxEYXRhLCAoKSk7CisgICAgcmV0dXJuICp0aHJlYWRHbG9iYWxEYXRhOwogI2Vsc2UK LSAgICBzdGF0aWMgVGhyZWFkR2xvYmFsRGF0YSogc3RhdGljRGF0YTsKLSAgICBpZiAoIXN0YXRp Y0RhdGEpIHsKLSAgICAgICAgc3RhdGljRGF0YSA9IHN0YXRpY19jYXN0PFRocmVhZEdsb2JhbERh dGEqPihmYXN0TWFsbG9jKHNpemVvZihUaHJlYWRHbG9iYWxEYXRhKSkpOwotICAgICAgICAvLyBU aHJlYWRHbG9iYWxEYXRhIGNvbnN0cnVjdG9yIGluZGlyZWN0bHkgdXNlcyBzdGF0aWNEYXRhLCBz byB3ZSBuZWVkIHRvIHNldCB1cCB0aGUgbWVtb3J5IGJlZm9yZSBpbnZva2luZyBpdC4KLSAgICAg ICAgbmV3IChzdGF0aWNEYXRhKSBUaHJlYWRHbG9iYWxEYXRhOwotICAgIH0KLSAgICByZXR1cm4g KnN0YXRpY0RhdGE7CisgICAgREVGSU5FX1NUQVRJQ19MT0NBTChUaHJlYWRHbG9iYWxEYXRhLCBz dGF0aWNEYXRhLCAoKSk7CisgICAgcmV0dXJuIHN0YXRpY0RhdGE7CiAjZW5kaWYKIH0KIAo=