Page MenuHomePhabricator

ecore/timer: correctly handle recursive deletion of legacy timers
ClosedPublic

Authored by zmike on Oct 28 2019, 12:53 PM.

Details

Summary

if a legacy timer callback returns false, the timer is deleted. in the
case where the legacy timer is deleted inside the callback while the same
timer is ticking recursively, however, the deletion must be deferred until
the outer-most frame of the timer's callstack has returned from the callback
in order to avoid improperly handling the timer

@fix

Depends on D10540

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
zmike created this revision.Oct 28 2019, 12:53 PM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

cedric requested changes to this revision.Oct 30 2019, 9:42 AM
cedric added inline comments.
src/lib/ecore/ecore_timer.c
220–221

I don't think it is necessary to maintain a duplicate anymore in the object data if we are to store it in the class structure. This or you always fetch the legacy pointer from the object data store.

One draw back/benefit is that with using the data store you do increase the access cost and the memory usage of legacy timer, but you decrease the memory usage of unified timer... No idea of which strategy is best. I will approve a patch that goes one way or the other.

This revision now requires changes to proceed.Oct 30 2019, 9:42 AM
zmike added inline comments.Oct 30 2019, 9:46 AM
src/lib/ecore/ecore_timer.c
220–221

The alternative was adding another pointer into the legacy struct. I don't care either way.

zmike updated this revision to Diff 26536.Oct 30 2019, 10:05 AM
zmike edited the summary of this revision. (Show Details)

remove data usage

cedric requested changes to this revision.Oct 30 2019, 10:33 AM
cedric added inline comments.
src/lib/ecore/ecore_timer.c
240

If you are testing for !td, I think you want to use efl_data_scope_safe_get. This would guarantee the right class being passed and the use of not crazy pointers.

This revision now requires changes to proceed.Oct 30 2019, 10:33 AM
zmike updated this revision to Diff 26560.Oct 30 2019, 11:43 AM

use safe data get

cedric accepted this revision.Oct 30 2019, 12:59 PM
This revision is now accepted and ready to land.Oct 30 2019, 12:59 PM
This revision was automatically updated to reflect the committed changes.