Hello again,

(for Brent Fulgham: codeword "onion", I repeat, codeword "onion" :))

Context
Consider embedding WebKit in an ASP.NET website (even though what follows is 
valid in other scenarios). In order to do this, you need a (managed) thread 
with a message loop. If you want this to work reliably, you make sure all 
WebKit's COM objects are created on that thread. If you chose not to, what 
follows is still valid.
Note that this thread doesn't need to be the main process thread: in the case 
of a worker process of a web server, you don't even have access to that. For 
WebKit, it just needs to be "a" thread with a message loop, as long as it's the 
same one.

When you instantiate a Webkit COM object for the first time, Windows' COM will 
ultimately call LoadLibrary on WebKit.dll. It will do so on the thread that 
wants to instantiate the COM object. This initializes the CRT and triggers a 
call to DllMain with the argument ul_reason_for_call set to DLL_PROCESS_ATTACH.
When the last reference to the last WebKit object is released, COM will NOT 
call FreeLibrary(). COM doesn't know the concept of "last object", and will 
keep the DLL around to avoid the overhead of loading it again should another 
instance be needed.
By default, your unmanaged DLL will remain loaded until the process is 
terminated. The main process thread will then call FreeLibrary(), prompting 
indirectly another call to DllMain with the argument ul_reason_for_call set to 
DLL_PROCESS_DETACH.
The main thing to take away from this, is that the thread executing 
DLL_PROCESS_ATTACH code and the thread executing DLL_PROCESS_DETACH code will 
not necessarily be the same. In the above scenario, they will never be the same.
For those who prefer the horse's mouth: 
http://msdn.microsoft.com/en-us/library/windows/desktop/ms682583(v=vs.85).aspx

AtomicString
Consider the following code:

LONG FontPlatformData::adjustedGDIFontWeight(LONG gdiFontWeight, const String& 
family)
{
    static AtomicString lucidaStr("Lucida Grande");
    if (equalIgnoringCase(family, lucidaStr)) {
        if (gdiFontWeight == FW_NORMAL)
            return FW_MEDIUM;
        if (gdiFontWeight == FW_BOLD)
            return FW_SEMIBOLD;
    }
    return gdiFontWeight;


This is a method (one of many examples) with a static AtomicString object. The 
constructor of lucidaStr will be executed in a thread-safe way (exactly once) 
by the first thread that will enter this method. The C++ compiler will generate 
code that will make sure of it, at least if the compiler obeys C++0x, which 
mandates this behavior.
Code will also be executed that registers the destructor  of lucidaStr so that 
it is executed when the program terminates. The mechanism is similar to 
functions registered with atexit(). Destruction order will be reverse 
construction order, as expected.
This code being in a DLL, it means that the destructor will be called when the 
DLL is unloaded: after the the DLL_PROCESS_DETACH code of DllMain, the CRT will 
do so.
Given the context described above, it means that the destructor of an 
AtomicString will be executed by a different thread than the one that executed 
its constructor.

When an AtomicString is destroyed, the following method is executed:

void AtomicString::remove(StringImpl* string)
{
    ASSERT(string->isAtomic());
    AtomicStringTableLocker locker;
    HashSet<StringImpl*>& atomicStringTable = stringTable();
    HashSet<StringImpl*>::iterator iterator = atomicStringTable.find(string);
    ASSERT_WITH_MESSAGE(iterator != atomicStringTable.end(), "The string being 
removed is atomic in the string table of an other thread!");
    atomicStringTable.remove(iterator);
}


This will fail on the assertion, since the string was created on the string 
table of a different thread.
The process will therefore crash on termination. Every time.

Why this is relevant
One can argue that for "eternal' processes, this is not relevant. 
Unfortunately, this is wrong. For example on Windows, IIS worker processes can 
be periodically recycled for legitimate reasons (elapsed or scheduled time, 
number of requests, memory usage or on demand). Every legitimate requests will 
log a process crash, which is bad form (to say the least).

Workaround
The workaround I've applied was to change the ASSERT_WITH_MESSAGE and the line 
below it in the above method to this:

    if(iterator != atomicStringTable.end())
      atomicStringTable.remove(iterator);

The workaround sucks because is never a good idea to eliminate a perfectly 
valid an assertion just for taking care of a corner case. Unfortunately, this 
corner case was systematic crashing behavior, which is worse.
I was afraid there were other classes whose static instances are so sensitive 
to threading, but it seems that AtomicString is the only one. Whew!
In my defense, I've seen worse. I've seen .NET wrappers for WebKit creating a 
static instance of WebView and call various methods in order to initialize the 
variables on the main thread and avoid the crash. Needless to say, these 
workarounds don't work.

Theoretical but impractical workaround 1
You can convince COM to unload unused libraries with a call to 
CoFreeUnusedLibraries(). This will trigger a call to DllCanUnloadNow (which is 
implemented by WebKit in WebKitDLL.cpp) and if that function returns S_OK (for 
WebKit this means: when there are no instances and no locks left), the DLL will 
be unloaded. Unfortunately, the question remains on which thread to call 
CoFreeUnusedLibraries? If you take the "multithreaded route" instantiating your 
objects, there is no right answer to that question, since different static 
atomic strings inside methods can be initialized on demand by different threads.
On .NET, there is also the problem of termination detection: usually, your 
managed "webkit thread" will be a background thread to avoid blocking the 
termination of the worker process. This means that the thread can be terminated 
without notice, and hence there is no good place to put termination code.

Theoretical but impractical workaround 2
You can forego all the built-in COM support, call LoadLibrary/FreeLibrary 
yourself., and handle all instantiating/calling/marshalling in .NET. Tricky and 
difficult, but not impossible.
But then, what's the point of providing COM support? And besides, calling 
FreeLibrary is as impractical as calling CoFreeUnusedLibraries() so you're back 
to square one.

AtomicString (again)
Unless I'm mistaken, an AtomicString instance is an immutable object, at least 
conceptually.  It makes a lot of sense for immutable objects (like the String 
class in managed environments Java and .NET) to be thread safe.
This is not the case for AtomicString because of some (thread) affinity to a 
thread-specific string table. This design doesn't work for static instances. 
For those instances, shouldn't AtomicString reference a global and 
thread-neutral string table? Perhaps via a special constructor to be used only 
in those instances?
I'm new here, so maybe the discussion is moot and there is some very good 
reason why things are designed the way they are. I just don't see it.  
Explanations are welcome.
The closest design I can compare it with is string interning 
(http://en.wikipedia.org/wiki/String_interning). But in managed languages, 
there's one big (thread-neutral and thread-safe) string table, not one string 
table per thread.

This is one of those cases where I feel WebKit's design conspires against you. 
Well, it certainly did against me. Now excuse me while I get my handkerchief.


Vincent

_______________________________________________
webkit-dev mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-dev

Reply via email to