Bug 37521 - [Qt] QtWebKit crash on shutdown
Summary: [Qt] QtWebKit crash on shutdown
Status: RESOLVED DUPLICATE of bug 36832
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Anders Bakken
URL:
Keywords: Qt
Depends on:
Blocks:
 
Reported: 2010-04-13 14:07 PDT by Anders Bakken
Modified: 2010-04-19 17:49 PDT (History)
6 users (show)

See Also:


Attachments
patch that fixes the crash (1.06 KB, patch)
2010-04-13 14:07 PDT, Anders Bakken
hausmann: review-
hausmann: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Bakken 2010-04-13 14:07:30 PDT
Created attachment 53278 [details]
patch that fixes the crash

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?
Comment 1 Zoltan Herczeg 2010-04-15 05:04:15 PDT
Is it a valgrind output?

Am I see right that QApplication destructor fires the shared timer system of WebCore? I am wondering the possible side effects of such call. Perhaps it would be better to delete the SharedTimer just before the destructor of QApplication...
Comment 2 Anders Bakken 2010-04-15 10:34:12 PDT
(In reply to comment #1)
> Is it a valgrind output?
> 
> Am I see right that QApplication destructor fires the shared timer system of
> WebCore? I am wondering the possible side effects of such call. Perhaps it
> would be better to delete the SharedTimer just before the destructor of
> QApplication...

Hi Zoltan

It is valgrind. I can't get gdb to run against a debug version of WebKit. I have to admit I wouldn't know about your proposed fix. I just saw the error and found a simple way to fix it. Maybe someone who knows the details better could comment.

regards

Anders
Comment 3 Simon Hausmann 2010-04-16 17:24:40 PDT
This is technically a duplicate of 36832, but I admit I like Anders' solution
Comment 4 Simon Hausmann 2010-04-16 17:24:59 PDT
I meant bug 36832
Comment 5 Zoltan Herczeg 2010-04-17 02:35:32 PDT
As far as I see this is an assertion fail, so only debug builds may crash. And the poblem is not there I think. Actually a half destroyed QApplication fires all timers, and a timer can do practically anything in WebKit, like it can also access/changes the members of this QApplication. This is unsafe. I feel this fix hides the issue, not solve it. Anyway it is just my opinion, nothing more.
Comment 6 Balazs Kelemen 2010-04-17 17:09:09 PDT
(In reply to comment #1)
> Is it a valgrind output?
> 
> Am I see right that QApplication destructor fires the shared timer system of
> WebCore? I am wondering the possible side effects of such call. Perhaps it
> would be better to delete the SharedTimer just before the destructor of
> QApplication...

Yep, this is exactly what happens here.
I think we can achieve the behavior what you mentioned by
installing a self-destroying slot for QApplication::aboutToQuit() into the
SharedTimerQt instance (and make it independent from the QApplication).
I will try it! :)
Comment 7 Simon Hausmann 2010-04-19 17:45:45 PDT

*** This bug has been marked as a duplicate of bug 36832 ***
Comment 8 Simon Hausmann 2010-04-19 17:49:10 PDT
Comment on attachment 53278 [details]
patch that fixes the crash

r- as http://trac.webkit.org/changeset/57818 was landed