Summary: | ThreadSpecific instances or ThreadGlobalData instance is being leaked when the WebKit library is being reloaded. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carol Szabo <carol> | ||||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED DUPLICATE | ||||||||
Severity: | Normal | CC: | ap, mrowe | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Carol Szabo
2009-10-15 14:49:04 PDT
Created attachment 41258 [details]
Proposed Patch
This addresses the singleton problem related to threadGlobalData().
Comment on attachment 41258 [details]
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.
Many aspects of the WebKit code are written with the assumption that the library will not be unloaded prior to exit. Created attachment 41310 [details]
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"
Comment on attachment 41310 [details] 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. |