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

Chris Dumez
Reported 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
Attachments
Patch (4.96 KB, patch)
2013-02-11 03:39 PST, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (4.98 KB, patch)
2013-02-11 03:41 PST, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2013-02-11 01:24:57 PST
I'll upload a patch soon.
Chris Dumez
Comment 2 2013-02-11 03:39:02 PST
Created attachment 187541 [details] Patch
Chris Dumez
Comment 3 2013-02-11 03:41:19 PST
Created attachment 187542 [details] Patch Add extra assertion to make sure we don't leak memory.
Raphael Kubo da Costa (:rakuco)
Comment 4 2013-02-11 03:55:38 PST
LGTM, good thing RunLoop is not in WK2 anymore :-)
Chris Dumez
Comment 5 2013-02-11 04:46:26 PST
Committed r142454: <http://trac.webkit.org/changeset/142454>
Note You need to log in before you can comment on or make changes to this bug.