[GTK] WebProcess crash on hitting assertion m_isWaitingForDidUpdate in DrawingAreaImpl

[Amruth Raj]

This is a crash happening sometimes during page load with the following backtrace.

Program terminated with signal 11, Segmentation fault.
#0  0x00007f3f7dae68f0 in WebKit::DrawingAreaImpl::display (this=0xe204e0) at ../../Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:560
560	    ASSERT(!m_isWaitingForDidUpdate);
(gdb) bt
#0  0x00007f3f7dae68f0 in WebKit::DrawingAreaImpl::display (this=0xe204e0) at ../../Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:560
#1  0x00007f3f7dae683e in WebKit::DrawingAreaImpl::displayTimerFired (this=0xe204e0) at ../../Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:552
#2  0x00007f3f7dae800e in Timer<WebKit::DrawingAreaImpl>::fired (this=0xe20570) at ../../Source/WebKit2/Platform/RunLoop.h:128
#3  0x00007f3f7d97602b in RunLoop::TimerBase::timerFiredCallback (timer=0xe20570) at ../../Source/WebKit2/Platform/gtk/RunLoopGtk.cpp:108
#4  0x00007f3f7a1b84eb in g_timeout_dispatch (source=0x135ff90, callback=<value optimized out>, user_data=<value optimized out>)
    at /build/buildd/glib2.0-2.28.6/./glib/gmain.c:3882
#5  0x00007f3f7a1b6bcd in g_main_dispatch (context=0xde0e60) at /build/buildd/glib2.0-2.28.6/./glib/gmain.c:2440
#6  g_main_context_dispatch (context=0xde0e60) at /build/buildd/glib2.0-2.28.6/./glib/gmain.c:3013
#7  0x00007f3f7a1b73a8 in g_main_context_iterate (context=0xde0e60, block=<value optimized out>, dispatch=1, self=<value optimized out>)
    at /build/buildd/glib2.0-2.28.6/./glib/gmain.c:3091
#8  0x00007f3f7a1b79f2 in g_main_loop_run (loop=0xddfb30) at /build/buildd/glib2.0-2.28.6/./glib/gmain.c:3299
#9  0x00007f3f7d975e0d in RunLoop::run () at ../../Source/WebKit2/Platform/gtk/RunLoopGtk.cpp:56
#10 0x00007f3f7da6d7ab in WebKit::WebProcessMainGtk (argc=2, argv=0x7fffc94b6eb8) at ../../Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:71
#11 0x00000000004007e4 in main (argc=2, argv=0x7fffc94b6eb8) at ../../Source/WebKit2/gtk/MainGtk.cpp:31

The usecase(though not consistent always) to reproduce this problem a little easily is:
Load http://hp.com in MiniBrowser
While the page keeps loading, keep on doing a maximize-unmaximize the window and it crashes in between.

I see the following sequence happening which is leading to a crash:
displayTimer callback gets triggered
UpdateInfo gets created and displayTimer is stopped in DrawingAreaImpl::display
WebProcess sends an Update event to the UIProcess
The acknowledgement from UI Process hasn't been sent yet
displayTimer callback in WebProcess gets triggered again and hits the ASSERT statement.

[Adam Roben (:aroben)]

(In reply to comment #0)
> displayTimer callback in WebProcess gets triggered again and hits the ASSERT statement.

Do you know when/why the display timer is firing again?

[Amruth Raj]

(In reply to comment #1)
> (In reply to comment #0)
> > displayTimer callback in WebProcess gets triggered again and hits the ASSERT statement.
> 
> Do you know when/why the display timer is firing again?
I am not sure. I will debug that. There are only a couple of places where displayTimer gets started. I'll put some prints and check it.

[Amruth Raj]

I found the problem to be with the GTK RunLoop::TimerBase implementation.

This happens when a timer gets created from another timerfired callback. In the present case, displayTimerFired does an m_displayTimer.startOneShot().

RunLoop::TimerBase maintains a single m_timerSource. This is causing a problem when trying to destroy the old timer which got fired. It is clearing the newly created timerSource.

I think the solution to this problem could be to pass the timerSource as a userdata that can be received in the callback function.

[Amruth Raj]

Created attachment 104972 [details]
Handle the case of a new timer getting created from another timer callback

Thanks to Martin for suggesting a simpler approach to fix this bug.
check-webkit-style was reporting a couple of errors in the files that I touched. With this patch, I have included fixes for these as well.

[Amruth Raj]

I'm listing the sequence of events again for more clarity:

RunLoop::TimerBase::timerFiredCallback gets called.

This calls timer->fired() resulting in a call to DrawingAreaImpl::displayTimerFired.

This creates a new one shot timer resulting in a call to RunLoop::TimerBase::start which overrides m_timerSource with a new value.

Next, the previous timer's destroyNotifyCallback gets called which sets the newly created m_timerSource to 0.

This results in the next timer never getting stopped even though a stop is called on it and eventually leads to a crash.

[Adam Roben (:aroben)]

Comment on attachment 104972 [details]
Handle the case of a new timer getting created from another timer callback

This looks fine to me, but it would probably be good for a GTK expert to have a look as well.

[Amruth Raj]

Created attachment 105002 [details]
Add description in ChangeLog

[WebKit Review Bot]

Comment on attachment 105002 [details]
Add description in ChangeLog

Clearing flags on attachment: 105002

Committed r93711: <http://trac.webkit.org/changeset/93711>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.