Page MenuHomePhabricator

eio: enforce proper lifecycle for all Efl.Io_Model and fix discovered lifecycle bugs.
ClosedPublic

Authored by cedric on Feb 1 2019, 4:00 AM.

Details

Summary

This make sure that the object returned by children_slice_get are properly
destroyed when the refcount drop to only the parent holding a reference on
it. This make it clear that the user of the api can rely on efl_ref/efl_unref
to actually manage its use of the returned object.

Additionnaly we are cleaning up the created object that we are using to build our own
request inside the Efl.Io.Model and avoid internal leak.

Depends on D7864

Diff Detail

Repository
rEFL core/efl
Branch
T7528-devs/cedric/child_noref
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 9635
cedric created this revision.Feb 1 2019, 4:00 AM
cedric updated this revision to Diff 19238.Feb 7 2019, 1:57 AM

Rebase and rename.

cedric updated this revision to Diff 19291.Feb 11 2019, 10:05 AM

Rebase and rename.

segfaultxavi resigned from this revision.Feb 21 2019, 12:37 PM

OK, this is far too complicated for my current knowledge. I humbly resign as a reviewer.

cedric updated this revision to Diff 19563.Feb 21 2019, 2:05 PM

Rebase.

zmike requested changes to this revision.Feb 21 2019, 2:08 PM

I'm gonna need a real commit log here.

This revision now requires changes to proceed.Feb 21 2019, 2:08 PM
cedric updated this revision to Diff 19566.Feb 21 2019, 2:41 PM
cedric edited the summary of this revision. (Show Details)

Rebase.

zmike requested changes to this revision.Feb 21 2019, 2:46 PM
zmike added inline comments.
src/lib/eio/efl_io_model.c
119

?

238

?

916

?

src/tests/eio/efl_io_model_test_monitor_add.c
61

This should be ck_abort_msg instead of fprintf+abort

This revision now requires changes to proceed.Feb 21 2019, 2:46 PM
cedric added inline comments.Feb 21 2019, 3:09 PM
src/lib/eio/efl_io_model.c
119

I find it a better practice to always use replace and make all the pointer NULL before calling free. Avoid access to dead pointer and weird stuff happening.

916

It was a misunderstanding of the API to think that child del was a synonym to efl_del, but it is not. It implies to destroy the underlying data that the model represent it. While efl_del imply only destroying the model.

cedric updated this revision to Diff 19570.Feb 21 2019, 6:01 PM

Rebase and use ck abort msg.

zmike requested changes to this revision.Feb 22 2019, 5:48 AM

one minor nit

src/lib/eio/efl_io_model.c
916

Oh okay. In that case, I was wondering why you had a return here for a void function?

This revision now requires changes to proceed.Feb 22 2019, 5:48 AM
zmike accepted this revision.Feb 23 2019, 5:56 AM

Alright I'm gonna hammer this in to expedite things.

This revision is now accepted and ready to land.Feb 23 2019, 5:56 AM
This revision was automatically updated to reflect the committed changes.