Bug 13553 – The static object in JSLazyEventListener::eventParameterName is destroyed after the static pthreads library is shut down

• Status: RESOLVED
• Resolution: INVALID
• Component: WebCore JavaScript

[Anyang Ren]

I'm using the nightly build WebKit-SVN-r21077 on Windows.
I am using pthreads-win32 as a static library.  Therefore
I have to call pthread_win32_process_detach_np() myself.
I build a DLL that contain WebKit and pthreads-win32 code.

The problem is that it is tricky to arrange for the Visual
C++ runtime to call pthread_win32_process_detach_np() after
it calls the destructor of the static object in
JSLazyEventListener::eventParameterName.  For example, if
I create a DllMain() function that calls
pthread_win32_process_detach_np(), pthread_win32_process_detach_np()
is called before the static object destructor is called.
This results in a crash in KJS:JSLock::lock() because it
does "free memory read" and "free memory write" (in Purify's
terminology).

I am not familiar with KJS code.
If I make this naive change to kjs_events.cpp to eliminate
the static object:

291,292c291
<     static ProtectedPtr<JSValue> eventString = jsString("event");
<     return eventString.get();
---
>     return jsString("event");

I can fix the crash.  I don't know if this patch is correct
or what its implications are.

[Alexey Proskuryakov]

Since static objects are destructed in an order opposite to their construction, I think you can use a destructor of a static object that is constructed early:

BOOL    WINAPI   DllMain (HANDLE hInst, 
                            ULONG ul_reason_for_call,
                            LPVOID lpReserved)
{
    switch (ul_reason_for_call) {
        case DLL_PROCESS_ATTACH: {
            static class StaticPthreadDetach {
                ~StaticPthreadDetach() { pthread_win32_process_detach_np(); }
            } staticPthreadDetach;
        break;
...
}

AFAIK, multiple threads are only needed in OS X (because the network loader uses JavaScriptCore to process proxy auto-configuration files), so I'm not sure why pthreads-win32 is required for the Win32 port to work.

[Anyang Ren]

I'm not sure if pthreads-win32 is required for the Win32 port to work.
I'm using it because it's there.

I think your solution will work (I didn't test it).  I came up with
an equivalent solution using _onexit that exploits the LIFO order of
atexit handlers.  But these solutions are subtle -- the naive
attempts of calling pthread_win32_process_detach_np in the
DLL_PROCESS_DETACH case of DllMain, or calling _onexit too late don't
work.  So I still encourage you to do something about this bug, but
since there are working solutions to this problem, you can close this
bug.

Thank you for your help!

[Anyang Ren]

I tested a variant of your suggestion in comment 1.  It works.

[Alexey Proskuryakov]

A static protected object is used here as a performance optimization, see <http://trac.webkit.org/projects/webkit/changeset/5404>. So, it doesn't look like there is an easy way to get rid of it. And I believe that it would be quite hard to ensure and subsequently maintain that pthread functions are not used during destruction of static objects, given that this has no negative consequences on OS X.

Between these two arguments, I'm going to close this bug.

[Anyang Ren]

Cool.  Have you considered the thread safety of the construction of
this function-local static object?  The C++ standard doesn't address
this issue, so how the function-local static object is contructed in
a multithreaded environment is implementation defined.

This issue is discussed in Matthew Wilson's "Imperfect C++", Section
11.3 "Function-Local Static Objects".

[Alexey Proskuryakov]

(In reply to comment #5)
> Cool.  Have you considered the thread safety of the construction of
> this function-local static object? 

WebCore is not thread-safe at all, so it is not a problem.

[Alexey Proskuryakov]

I should have also mentioned that GCC's static object construction thread safety is explicitly disabled by compiler flags in our Xcode WebCore project (for Mac) for better performance.

[Anyang Ren]

Thanks for the info, Alexey.  The reason I brought up the issue
of thread safety was that the constructors and the = operator
for that static ProtectedPtr<JSValue> object all acquire a lock.
So I thought we also had to ensure that the static object's
construction/initialization is only done once in a multithreaded
environment.