Bug 30408 - ThreadSpecific instances or ThreadGlobalData instance is being leaked when the WebKit library is being reloaded.
Summary: ThreadSpecific instances or ThreadGlobalData instance is being leaked when th...
Status: RESOLVED DUPLICATE of bug 27980
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2009-10-15 14:49 PDT by Carol Szabo
Modified: 2009-10-16 14:44 PDT (History)
2 users (show)

See Also:

Proposed Patch (2.06 KB, patch)
2009-10-15 18:18 PDT, Carol Szabo
ap: review-
Details | Formatted Diff | Diff
Proposed Patch (3.39 KB, patch)
2009-10-16 12:48 PDT, Carol Szabo
ap: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carol Szabo 2009-10-15 14:49:04 PDT
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.
Comment 1 Carol Szabo 2009-10-15 18:18:55 PDT
Created attachment 41258 [details]
Proposed Patch

This addresses the singleton problem related to threadGlobalData().
Comment 2 Alexey Proskuryakov 2009-10-15 18:57:38 PDT
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.
Comment 3 Mark Rowe (bdash) 2009-10-15 23:29:05 PDT
Many aspects of the WebKit code are written with the assumption that the library will not be unloaded prior to exit.
Comment 4 Carol Szabo 2009-10-16 12:48:05 PDT
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 5 Alexey Proskuryakov 2009-10-16 14:43:57 PDT
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.
Comment 6 Alexey Proskuryakov 2009-10-16 14:44:08 PDT

*** This bug has been marked as a duplicate of bug 27980 ***