[webkit-reviews] review requested: [Bug 37521] QtWebKit crash on shutdown : [Attachment 53278] patch that fixes the crash

[bugzilla-daemon at webkit.org]

Anders Bakken <agbakken at gmail.com> has asked  for review:
Bug 37521: QtWebKit crash on shutdown
https://bugs.webkit.org/show_bug.cgi?id=37521

Attachment 53278: patch that fixes the crash
https://bugs.webkit.org/attachment.cgi?id=53278&action=review

------- Additional Comments from Anders Bakken <agbakken at gmail.com>
I get the following crash on shutdown of a Qt webkit application:

==8808== Invalid read of size 8
==8808==    at 0x9AF2E90: QObject::thread() const (qobject.cpp:1409)
==8808==    by 0x72801BD: WTF::isMainThread() (ThreadingQt.cpp:220)
==8808==    by 0x66C7ED5: WebCore::JSDOMWindowBase::commonJSGlobalData()
(JSDOMWindowBase.cpp:155)
==8808==    by 0x66AF816: WebCore::collect(void*) (GCController.cpp:46)
==8808==    by 0x66AFADF:
WebCore::GCController::gcTimerFired(WebCore::Timer<WebCore::GCController>*)
(GCController.cpp:69)
==8808==    by 0x66AFC33: WebCore::Timer<WebCore::GCController>::fired()
(Timer.h:98)
==8808==    by 0x6CA0A53: WebCore::ThreadTimers::sharedTimerFiredInternal()
(ThreadTimers.cpp:112)
==8808==    by 0x6CA0986: WebCore::ThreadTimers::sharedTimerFired()
(ThreadTimers.cpp:90)
==8808==    by 0x6E73E36: WebCore::SharedTimerQt::~SharedTimerQt()
(SharedTimerQt.cpp:68)
==8808==    by 0x9AF59D6: QObjectPrivate::deleteChildren() (qobject.cpp:1972)
==8808==    by 0x9AF8BFB: QObject::~QObject() (qobject.cpp:969)
==8808==    by 0x8E7DCB3: QApplication::~QApplication() (qapplication.cpp:1138)

==8808==  Address 0x8 is not stack'd, malloc'd or (recently) free'd
==8808== 
==8808== 
==8808== Process terminating with default action of signal 11 (SIGSEGV):
dumping core
==8808==  Access not within mapped region at address 0x8
==8808==    at 0x9AF2E90: QObject::thread() const (qobject.cpp:1409)
==8808==    by 0x72801BD: WTF::isMainThread() (ThreadingQt.cpp:220)
==8808==    by 0x66C7ED5: WebCore::JSDOMWindowBase::commonJSGlobalData()
(JSDOMWindowBase.cpp:155)
==8808==    by 0x66AF816: WebCore::collect(void*) (GCController.cpp:46)
==8808==    by 0x66AFADF:
WebCore::GCController::gcTimerFired(WebCore::Timer<WebCore::GCController>*)
(GCController.cpp:69)
==8808==    by 0x66AFC33: WebCore::Timer<WebCore::GCController>::fired()
(Timer.h:98)
==8808==    by 0x6CA0A53: WebCore::ThreadTimers::sharedTimerFiredInternal()
(ThreadTimers.cpp:112)
==8808==    by 0x6CA0986: WebCore::ThreadTimers::sharedTimerFired()
(ThreadTimers.cpp:90)
==8808==    by 0x6E73E36: WebCore::SharedTimerQt::~SharedTimerQt()
(SharedTimerQt.cpp:68)
==8808==    by 0x9AF59D6: QObjectPrivate::deleteChildren() (qobject.cpp:1972)
==8808==    by 0x9AF8BFB: QObject::~QObject() (qobject.cpp:969)
==8808==    by 0x8E7DCB3: QApplication::~QApplication() (qapplication.cpp:1138)


It reproduces pretty much every time but the example is rather involved and
includes adding some code to WebKit that may or may not be important for
reproducing the problem. Let me know if you want to see the example. 

It seems like the problem is that isMainThread is called after QCoreApplication
is deleted. 

I've attached a patch that fixes it but I am not 100% about the following:

Could this thread be called in one thread while QCoreApplication is being
deleted in another? If so, one would have to do some more
thread-synchronization for this to be safe.


Should it return true or false if QCoreApplication doesn't exist?