Bug 109409

Summary: [EFL] Stop using smart pointers for Ecore_Timer
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit EFLAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, bw80.lee, cmarcelo, d-r, gyuyoung.kim, kenneth, laszlo.gombos, lucas.de.marchi, ojan.autocc, rakuco, tmpsantos, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://docs.enlightenment.org/auto/ecore/group__Ecore__Timer__Group.html#ga501bb88fb0b5ad134b78f489c879099f
Bug Depends on:    
Bug Blocks: 109407    
Attachments:
Description Flags
Patch
none
Patch none

Description Chris Dumez 2013-02-11 01:15:00 PST 
Using smart pointers (WTF::OwnPtr in this case) for Ecore_Timer is a bad idea because the timer handle becomes invalid as soon as the timer's callback returns ECORE_CALLBACK_CANCEL. ecore_timer_del() should NOT be called on an invalid timer handle.

This is used in RunLoopEfl.cpp and may lead to crashes such as:
STDOUT: <empty>
STDERR: 1   0x7f6e7bdf094f
STDERR: 2   0x7f6e7eb6b4a0
STDERR: 3   0x7f6e77aa1f80 ecore_timer_del
STDERR: 4   0x7f6e7be23a83 WTF::deleteOwnedPtr(_Ecore_Timer*)
STDERR: 5   0x7f6e7bcd4612 WTF::OwnPtr<_Ecore_Timer>::clear()
STDERR: 6   0x7f6e7bcd43a0 WTF::OwnPtr<_Ecore_Timer>::operator=(std::nullptr_t)
STDERR: 7   0x7f6e7bcd40d8 WebCore::RunLoop::TimerBase::timerFired(void*)
STDERR: 8   0x7f6e77aa23de _ecore_timer_expired_call
STDERR: 9   0x7f6e77aa25ab _ecore_timer_expired_timers_call
STDERR: 10  0x7f6e77a9f4b1
STDERR: 11  0x7f6e77a9fb47 ecore_main_loop_begin
STDERR: 12  0x7f6e7bcd3ea7 WebCore::RunLoop::run()
STDERR: 13  0x7f6e7fa7036d WebProcessMainEfl
STDERR: 14  0x400804 main
STDERR: 15  0x7f6e7eb5676d __libc_start_main
STDERR: 16  0x400729
STDERR: LEAK: 1 WebPage

or 

crash log for WebKitTestRunner (pid 7660):
STDOUT: <empty>
STDERR: 6   0x7fa347f8bf28 WTF::OwnPtr<_Ecore_Timer>::operator=(std::nullptr_t)
STDERR: 7   0x7fa347f8bc60 WebCore::RunLoop::TimerBase::timerFired(void*)
STDERR: 8   0x7fa343d543de _ecore_timer_expired_call
STDERR: 9   0x7fa343d545ab _ecore_timer_expired_timers_call
STDERR: 10  0x7fa343d514b1
STDERR: 11  0x7fa343d51b47 ecore_main_loop_begin
STDERR: 12  0x7fa347f8ba2f WebCore::RunLoop::run()
STDERR: 13  0x7fa34bd254f5 WebProcessMainEfl
STDERR: 14  0x400804 main
STDERR: 15  0x7fa34ae0b76d __libc_start_main
STDERR: 16  0x400729
Comment 1 Chris Dumez 2013-02-11 01:24:57 PST 
I'll upload a patch soon.
Comment 2 Chris Dumez 2013-02-11 03:39:02 PST 
Created attachment 187541 [details]
Patch
Comment 3 Chris Dumez 2013-02-11 03:41:19 PST 
Created attachment 187542 [details]
Patch

Add extra assertion to make sure we don't leak memory.
Comment 4 Raphael Kubo da Costa (:rakuco) 2013-02-11 03:55:38 PST 
LGTM, good thing RunLoop is not in WK2 anymore :-)
Comment 5 Chris Dumez 2013-02-11 04:46:26 PST 
Committed r142454: <http://trac.webkit.org/changeset/142454>