Bug 34081

Summary: [Qt] QtWebkit apps crash on exit on Symbian^3 onwards
Product: WebKit Reporter: Janne Koskinen <koshuin>
Component: New BugsAssignee: Janne Koskinen <koshuin>
Status: CLOSED FIXED    
Severity: Normal CC: commit-queue, hausmann, laszlo.gombos, zherczeg
Priority: P1 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: S60 Hardware   
OS: S60 3rd edition   
Bug Depends on:    
Bug Blocks: 27065, 34751, 35784    
Attachments:
Description Flags
call-stack of the crash
none
proposed fix for webcore/config.h none

Description Janne Koskinen 2010-01-25 04:01:12 PST
Symbian^3 is the first version to implement global destructors and there is kern-exec 3 crash coming when running the destructors (see attached call-stack).

There is something really fishy going on with __cxa_finalize as you can see we end up calling WebCore::threadGlobalData() construction during the destruction which leads to crash on WTF::isMainThread() where there is no instance of QtCoreApp anymore.
Offending call is ThreadingQt.cpp::IsMainThread() { ... QCoreApplication::instance()->thread()}

Symbian does a lazy unloading of DLLs and the crash happens long after the application has exited (~30sec) and nothing is alive at that point. So my suggested hotfix for this one is to have:

bool isMainThread()
{
+    if (QCoreApplication::instance() == 0)
+        return true;
    return QThread::currentThread() == QCoreApplication::instance()->thread();
}

Above has been tested and it works. I'd still like to see the root cause why this is happening.
Comment 1 Janne Koskinen 2010-01-25 04:02:16 PST
Created attachment 47334 [details]
call-stack of the crash
Comment 2 Janne Koskinen 2010-02-26 00:17:52 PST
Now that I got enviroment to work on found the reason for the crash. fix might take some time....

Problem is that when we exit the application we delete the ThreadGlobalData while in main:

Thread [Thread id: 7392] (Suspended: Breakpoint hit.)	
	21 WebCore::ThreadGlobalData::~ThreadGlobalData() X:\qt\src\3rdparty\webkit\WebCore\platform\ThreadGlobalData.cpp:88 0x07f9a780	
	20 WTF::ThreadSpecific<WebCore::ThreadGlobalData>::destroy() X:\qt\src\3rdparty\webkit\JavaScriptCore\wtf\ThreadSpecific.h:226 0x07f9a0ec	
	19 WTF::ThreadSpecific<WebCore::ThreadGlobalData>::Data::~Data() X:\qt\src\3rdparty\webkit\JavaScriptCore\wtf\ThreadSpecific.h:83 0x07f9a552	
	18 0x7F9A50C( QtWebKit.dll )()  0x07f9a50c	
	17 QThreadStorage<WTF::ThreadSpecific<WebCore::ThreadGlobalData>::Data *>::deleteData() X:\qt\src\corelib\thread\qthreadstorage.h:133 0x07f9a4cd	
	16 QThreadStorageData::finish() X:\qt\src\corelib\thread\qthreadstorage.cpp:185 0x035417d3	
	15 QCoreApplicationPrivate::~QCoreApplicationPrivate() X:\qt\src\corelib\kernel\qcoreapplication.cpp:289 0x034dea2c	
	14 QApplicationPrivate::~QApplicationPrivate() X:\qt\src\gui\kernel\qapplication.cpp:192 0x03f7a8de	
	13 0x3F887AC( QtGui.dll )()  0x03f887ac	
	12 QScopedPointerDeleter<QObjectData>::cleanup() X:\qt\src\corelib\tools\qscopedpointer.h:63 0x034f5428	
	11 QScopedPointer<QObjectData, QScopedPointerDeleter<QObjectData>>::~QScopedPointer() X:\qt\src\corelib\tools\qscopedpointer.h:100 0x034f53f5	
	10 QObject::~QObject() X:\qt\src\corelib\kernel\qobject.cpp:984 0x034f5b41	
	9 QCoreApplication::~QCoreApplication() X:\qt\src\corelib\kernel\qcoreapplication.cpp:618 0x034df624	
	8 QApplication::~QApplication() X:\qt\src\gui\kernel\qapplication.cpp:1115 0x03f7c6ea	
	7 main() X:\webviewclient\main.cpp:56 0x00401150	
	6 QtMainWrapper() X:\qt\src\s60main\qts60main_mcrt0.cpp:86 0x00403201	
	5 E32Main() X:\qt\src\s60main\qts60main.cpp:56 0x00402d7c	
	4 _E32Startup() M:\sf\os\kernelhwsrv\kernel\eka\euser\epoc\win32\uc_exe.cpp:87 0x004024dc	
	3 DThread::EpocThreadFunction() V:\sf\os\KERNELHWSRV\KERNEL\EKA\KERNEL\win32\ckernel.cpp:84 0x0915ac33	
	2 NThread::StartThread() V:\sf\os\KERNELHWSRV\KERNEL\EKA\nkern\win32\ncthrd.cpp:187 0x091595c7	
	1 0x7C80B50B( KERNEL32.dll )()  0x7c80b50b	

And now that we hit the global destructors we get:

Thread [Thread id: 7392] (Suspended: Breakpoint hit.)	
	21 WTF::ThreadSpecific<WebCore::ThreadGlobalData>::operator WebCore::ThreadGlobalData *() X:\qt\src\3rdparty\webkit\JavaScriptCore\wtf\ThreadSpecific.h:251 0x07f9a255	
	20 WTF::ThreadSpecific<WebCore::ThreadGlobalData>::operator *() X:\qt\src\3rdparty\webkit\JavaScriptCore\wtf\ThreadSpecific.h:267 0x07f9a209	
	19 WebCore::threadGlobalData() X:\qt\src\3rdparty\webkit\WebCore\platform\ThreadGlobalData.cpp:58 0x07f9a1c2	
	18 WebCore::stringTable() X:\qt\src\3rdparty\webkit\WebCore\platform\text\AtomicString.cpp:45 0x07ff1698	
	17 WebCore::AtomicString::remove() X:\qt\src\3rdparty\webkit\WebCore\platform\text\AtomicString.cpp:229 0x07ff1b3c	
	16 WebCore::StringImpl::~StringImpl() X:\qt\src\3rdparty\webkit\WebCore\platform\text\StringImpl.cpp:104 0x07ffaf39	
	15 0x790B5AC( QtWebKit.dll )()  0x0790b5ac	
	14 WTF::RefCounted<WebCore::StringImpl>::deref() X:\qt\src\3rdparty\webkit\JavaScriptCore\wtf\RefCounted.h:110 0x0790b567	
	13 WTF::derefIfNotNull<WebCore::StringImpl>() X:\qt\src\3rdparty\webkit\JavaScriptCore\wtf\PassRefPtr.h:43 0x0790b531	
	12 WTF::RefPtr<WebCore::StringImpl>::~RefPtr() X:\qt\src\3rdparty\webkit\JavaScriptCore\wtf\RefPtr.h:53 0x0790b195	
	11 0x790B169( QtWebKit.dll )()  0x0790b169	
	10 0x790D7A9( QtWebKit.dll )()  0x0790d7a9	
	9 __destroy_global_chain() M:\sf\os\kernelhwsrv\kernel\eka\include\win32atx.h:96 0x08239d56	
	8 invokeTable() M:\sf\os\kernelhwsrv\kernel\eka\include\win32crt.h:74 0x0823a5f3	
	7 _E32Dll() M:\sf\os\kernelhwsrv\kernel\eka\euser\epoc\win32\uc_dll.cpp:73 0x0823a66b	
	6 CallStaticEntryPoints() V:\sf\os\kernelhwsrv\kernel\eka\euser\us_func.cpp:767 0x60014ae0	
	5 User::Exit() V:\sf\os\kernelhwsrv\kernel\eka\euser\us_func.cpp:806 0x60014b56	
	4 _E32Startup() M:\sf\os\kernelhwsrv\kernel\eka\euser\epoc\win32\uc_exe.cpp:93 0x004024f9	
	3 DThread::EpocThreadFunction() V:\sf\os\KERNELHWSRV\KERNEL\EKA\KERNEL\win32\ckernel.cpp:84 0x0915ac33	
	2 NThread::StartThread() V:\sf\os\KERNELHWSRV\KERNEL\EKA\nkern\win32\ncthrd.cpp:187 0x091595c7	
	1 0x7C80B50B( KERNEL32.dll )()  0x7c80b50b	

This is because of
static inline HashSet<StringImpl*>& stringTable()
{
    return threadGlobalData().atomicStringTable();
}
ends up constructing the threadGlobalData as there isn't one anymore as it was already deleted in main.
Comment 3 Janne Koskinen 2010-03-15 01:56:03 PDT
So what do you guys think about this ? The issue in short is that we use StringImpl as globals when it has a dependency into TLS. Either StringImpl must not be declared as global or the dependency to TLS must be removed.
Comment 4 Zoltan Herczeg 2010-03-16 02:42:18 PDT
(In reply to comment #3)
> So what do you guys think about this ? The issue in short is that we use
> StringImpl as globals when it has a dependency into TLS. Either StringImpl must
> not be declared as global or the dependency to TLS must be removed.

Interesting. Actually I fixed the free method of ThreadGlobalData on Qt, since it was leaking when Workers (runs JavaSCript in a thread) were involved.

WebKit has a policy to avoid global constructors and destructors at all. Global objects must be created with placement new (ugly) or they must be pointers to the global objects.

This is also a common method:

static object& get_object() {
    static object o;
    return &o;
}

According to the back-trace a string destructor caused the error:

    12 WTF::RefPtr<WebCore::StringImpl>::~RefPtr()
X:\qt\src\3rdparty\webkit\JavaScriptCore\wtf\RefPtr.h:53 0x0790b195

I think someone just defined a plain AtomicString somewhere (probably Qt only). Since such atomic strings are part of the StringTable, they have already been freed. But the C++ doesn't know about that.
Comment 5 Janne Koskinen 2010-03-17 03:25:47 PDT
(In reply to comment #4)
> (In reply to comment #3)
> I think someone just defined a plain AtomicString somewhere (probably Qt only).
> Since such atomic strings are part of the StringTable, they have already been
> freed. But the C++ doesn't know about that.

Thanks. Based on our IRC discussion I enabled SKIP_STATIC_CONSTRUCTORS_ON_GCC for Symbian but it didn't have the desired effect. When enabled QtWebkit stops linking on Symbian. All QualifiedNames become undefined e.g.

   mwldsym2.exe: Undefined symbol: 'class WebCore::QualifiedName const WebCore::
HTMLNames::aria_valuenowAttr (?aria_valuenowAttr@HTMLNames@WebCore@@3VQualifiedN
ame@2@B)'
   mwldsym2.exe: referenced from 'void WebCore::Element::updateAfterAttributeCha
nged(class WebCore::Attribute *) (?updateAfterAttributeChanged@Element@WebCore@@
IAEXPAVAttribute@2@@Z)' in Element.cpp:587
   mwldsym2.exe: Undefined symbol: 'class WebCore::QualifiedName const WebCore::
HTMLNames::labelTag (?labelTag@HTMLNames@WebCore@@3VQualifiedName@2@B)'
   mwldsym2.exe: referenced from 'class WTF::HashSet<class WebCore::AtomicString
Impl *, struct WTF::PtrHash<class WebCore::AtomicStringImpl *>, struct WTF::Hash
Traits<class WebCore::AtomicStringImpl *> > * WebCore::inlineTagList(void) (?inl
ineTagList@WebCore@@YAPAV?$HashSet@PAVAtomicStringImpl@WebCore@@U?$PtrHash@PAVAt
omicStringImpl@WebCore@@@WTF@@U?$HashTraits@PAVAtomicStringImpl@WebCore@@@4@@WTF
@@XZ)' in HTMLElement.cpp:838

I'll keep digging :)
Comment 6 Janne Koskinen 2010-03-23 10:36:20 PDT
Addition to be blocker for webkit release. Today I compiled the whole thing for ARM with SKIP_STATIC_CONSTRUCTORS_ON_GCC enabled and it seems to do the trick. It is only WINSCW that won't work, I'll prepare a patch tomorrow once I've tested a range of devices.
Comment 7 Janne Koskinen 2010-03-26 06:21:52 PDT
Created attachment 51735 [details]
proposed fix for webcore/config.h

Patch tested on Symbian^3 based OEM hardware.
Emulator will still continue crashing on exit but I think that should be ok. I tried emulator using same defines as for Windows build but
the WINSCW compiler seems to be too picky about the extern - keyword for class declarations. I think this is good compromise If wanted the workaround in bugreport comments can be applied for emulator environment.
Comment 8 Laszlo Gombos 2010-03-26 07:08:23 PDT
Comment on attachment 51735 [details]
proposed fix for webcore/config.h

Looks good to me. r+.
Comment 9 WebKit Commit Bot 2010-03-26 10:04:14 PDT
Comment on attachment 51735 [details]
proposed fix for webcore/config.h

Clearing flags on attachment: 51735

Committed r56631: <http://trac.webkit.org/changeset/56631>
Comment 10 WebKit Commit Bot 2010-03-26 10:04:19 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Simon Hausmann 2010-03-28 02:38:15 PDT
Shall I also backport this patch to Qt 4.6?
Comment 12 Simon Hausmann 2010-03-28 14:45:43 PDT
Revision r56631 cherry-picked into qtwebkit-2.0 with commit 88e5ac629c848e7a8fcaa9fa8b4181630c6836c0
Comment 13 Janne Koskinen 2010-03-29 01:24:36 PDT
(In reply to comment #11)
> Shall I also backport this patch to Qt 4.6?

Yes I think it should be backported due to significant impact on WSD size.
Comment 14 Simon Hausmann 2010-03-29 04:04:31 PDT
Cherry-picked into qtwebkit-4.6 with commit 9e0708dd02ed0feb22ba815c9def7fdfc53dd68e