Web Inspector: Tests crash on Qt bots revealed by r66720

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Csaba Osztrogonác]

After http://trac.webkit.org/changeset/66720 there are
some flakey crashes on Qt Linux Release bot. :/

Fortunately I reproduced the crash on our 64 bit relase
and debug bots, where 50-60 tests crash always.

$ WebKitTools/Scripts/run-webkit-tests --debug -exit-after-n-crashes 1

fast/dom/location-new-window-no-crash.html -> crashed
Exiting early after 1 crashes and 0 timeouts. 4946 tests run.

$ gdb WebKitBuild/Debug/bin/DumpRenderTree core

#0  0x00007fd89917f702 in WebCore::InspectorFrontendClientQt::inspectorClientDestroyed (this=0x908e5a0a39be2ed) at ../../../WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:344
344         m_inspectorClient = 0;
(gdb) bt
#0  0x00007fd89917f702 in WebCore::InspectorFrontendClientQt::inspectorClientDestroyed (this=0x908e5a0a39be2ed) at ../../../WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:344
#1  0x00007fd89917f73e in WebCore::InspectorClientQt::inspectorDestroyed (this=0x739810) at ../../../WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:104
#2  0x00007fd898d3e9f6 in WebCore::InspectorController::inspectedPageDestroyed (this=0x739b30) at ../../../WebCore/inspector/InspectorController.cpp:212
#3  0x00007fd898eadab4 in ~Page (this=0x739840) at ../../../WebCore/page/Page.cpp:211
#4  0x00007fd8991a9357 in ~QWebPagePrivate (this=0x739350) at ../../../WebKit/qt/Api/qwebpage.cpp:319
#5  0x00007fd8991a98be in ~QWebPage (this=0x7391d0) at ../../../WebKit/qt/Api/qwebpage.cpp:1891
#6  0x0000000000428de3 in ~WebPage (this=0x7391d0) at /home/oszi/WebKit/WebKitTools/DumpRenderTree/qt/DumpRenderTreeQt.cpp:168
#7  0x00007fd8991abf9a in QWebViewPrivate::detachCurrentPage (this=0x6f2080) at ../../../WebKit/qt/Api/qwebview.cpp:372
#8  0x00007fd8991accd9 in ~QWebViewPrivate (this=0x6f2080) at ../../../WebKit/qt/Api/qwebview.cpp:60
#9  0x00007fd8991acdce in ~QWebView (this=0x738cc0) at ../../../WebKit/qt/Api/qwebview.cpp:329
#10 0x0000000000425b73 in ~DumpRenderTree (this=0x7fffffffea80) at /home/oszi/WebKit/WebKitTools/DumpRenderTree/qt/DumpRenderTreeQt.cpp:505
#11 0x000000000043b0a3 in main (argc=2, argv=0x7fffffffecb8) at /home/oszi/WebKit/WebKitTools/DumpRenderTree/qt/main.cpp:171

Yury, Pavel, any idea how to fix it?

[Csaba Osztrogonác]

Additionally I tried to rollout http://trac.webkit.org/changeset/66720 locally, and after that I didn't get any crashes.

Guys, could we fix it quickly? Or should we rollout 
until fix make sheriffbot not to SPAM innocent peoples.

[Yury Semikhatsky]

(In reply to comment #1)
> Additionally I tried to rollout http://trac.webkit.org/changeset/66720 locally, and after that I didn't get any crashes.
> 
> Guys, could we fix it quickly? Or should we rollout 
> until fix make sheriffbot not to SPAM innocent peoples.

Let me look into it. It didn't crash locally. It was 32 bit virtual machine though.

[Csaba Osztrogonác]

(In reply to comment #2)
> Let me look into it. It didn't crash locally. It was 32 bit virtual machine though.

Qt Linux Release bot is a 32 bit machine too. Unfortunately
it isn't trivial to reproduce on it, because sometimes works,
sometimes not. See http://build.webkit.org/waterfall?show=Qt%20Linux%20Release

That's why sheriffbot make some SPAM.

[Yury Semikhatsky]

(In reply to comment #3)
> (In reply to comment #2)
> > Let me look into it. It didn't crash locally. It was 32 bit virtual machine though.
> 
> Qt Linux Release bot is a 32 bit machine too. Unfortunately
> it isn't trivial to reproduce on it, because sometimes works,
> sometimes not. See http://build.webkit.org/waterfall?show=Qt%20Linux%20Release
> 
> That's why sheriffbot make some SPAM.

Ok, it seems that destructor ~InspectorFrontendClientQt is called without preceding call to InspectorFrontendClientQt::destroyInspectorView. I'm going to commit a patch that should prove it.

[Antonio Gomes]

It also affects the QtTestBrowser (QtWebkit's launcher)

[Csaba Osztrogonác]

(In reply to comment #4)
> Ok, it seems that destructor ~InspectorFrontendClientQt is called without preceding call to InspectorFrontendClientQt::destroyInspectorView. I'm going to commit a patch that should prove it.

Landed in http://trac.webkit.org/changeset/66824 , but unfortunately still crash.

[Csaba Osztrogonác]

I can reproduced it on 32 bit too:

#0  0xf676a9c8 in WebCore::InspectorFrontendClientQt::inspectorClientDestroyed (this=0xffffffff) at ../../../WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:351
351         m_inspectorClient = 0;
(gdb) bt
#0  0xf676a9c8 in WebCore::InspectorFrontendClientQt::inspectorClientDestroyed (this=0xffffffff) at ../../../WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:351
#1  0xf676aa06 in WebCore::InspectorClientQt::inspectorDestroyed (this=0x815f018) at ../../../WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:104
#2  0xf631b5a7 in WebCore::InspectorController::inspectedPageDestroyed (this=0x81691c8) at ../../../WebCore/inspector/InspectorController.cpp:212
#3  0xf648afd5 in ~Page (this=0x8168ad0) at ../../../WebCore/page/Page.cpp:211
#4  0xf6794c3a in ~QWebPagePrivate (this=0x8168df0) at ../../../WebKit/qt/Api/qwebpage.cpp:319
#5  0xf6795152 in ~QWebPage (this=0xf27017e0) at ../../../WebKit/qt/Api/qwebpage.cpp:1891
#6  0x0806948c in ~WebPage (this=0xf27017e0) at /home/oszi/WebKit/WebKitTools/DumpRenderTree/qt/DumpRenderTreeQt.cpp:168
#7  0xf679759e in QWebViewPrivate::detachCurrentPage (this=0xf2700538) at ../../../WebKit/qt/Api/qwebview.cpp:372
#8  0xf67982d7 in ~QWebViewPrivate (this=0xf2700538) at ../../../WebKit/qt/Api/qwebview.cpp:60
#9  0xf67983de in ~QWebView (this=0xf27005c0) at ../../../WebKit/qt/Api/qwebview.cpp:329
#10 0x080664b3 in ~DumpRenderTree (this=0xffffc614) at /home/oszi/WebKit/WebKitTools/DumpRenderTree/qt/DumpRenderTreeQt.cpp:505
#11 0x0807a8b8 in main (argc=2, argv=0xffffc744) at /home/oszi/WebKit/WebKitTools/DumpRenderTree/qt/main.cpp:171

It seems m_frontendClient is invalid for some reason. (this=0xffffffff)

void InspectorClientQt::inspectorDestroyed()
{
    if (m_frontendClient)
        m_frontendClient->inspectorClientDestroyed(); 
        ---> crash!!! (InspectorClientQt.cpp:104)
    delete this;
}

[Csaba Osztrogonác]

Created attachment 66652 [details]
proposed fix

I tested locally, it fixed all crashes

[Antonio Gomes]

Comment on attachment 66652 [details]
proposed fix

r=me

[Csaba Osztrogonác]

Comment on attachment 66652 [details]
proposed fix

Clearing flags on attachment: 66652

Committed r66835: <http://trac.webkit.org/changeset/66835>

[Csaba Osztrogonác]

All reviewed patches have been landed.  Closing bug.

[Yury Semikhatsky]

(In reply to comment #11)
> All reviewed patches have been landed.  Closing bug.

Thanks for fixing this! The idea of missing initializer just came to my mind when I was driving home. I wonder why there is no such warning in g++.