Page MenuHomePhabricator

eio: rely on efl_future_then to properly protect Eo object during the lifecycle of the future callback.
AcceptedPublic

Authored by cedric on Wed, Mar 13, 2:58 PM.

Diff Detail

Repository
rEFL core/efl
Branch
devs/cedric/fileselector
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 10509
cedric created this revision.Wed, Mar 13, 2:58 PM
cedric updated this revision to Diff 20646.Fri, Mar 15, 4:58 PM
cedric added a reviewer: YOhoho.

Rebase.

bu5hm4n added inline comments.Sat, Mar 16, 8:42 AM
src/lib/eio/efl_io_model.c
455

Just for me as a understanding point, this here could actaully also return NULL, right ?

cedric added inline comments.Sat, Mar 16, 10:30 AM
src/lib/eio/efl_io_model.c
455

It will always return an Eina_Value. Ideally one with a future, but if it can't create it, it will be an EINA_VALUE_EMPTY.

bu5hm4n added inline comments.Sat, Mar 16, 10:53 AM
src/lib/eio/efl_io_model.c
455

Okay, my question more clear: This future here in a eina value can also be just something else, since this future will not be assosiated with the promise that is resolved so _eio_build_mime_now is executed.

cedric added inline comments.Sat, Mar 16, 11:01 AM
src/lib/eio/efl_io_model.c
455

So it does just return a new future that will be resolved later using another job. The future as value actually is understood by the logic of Eina_Future and automatically plug this new future in the chain and will wait on it to resolve before continuing the chain. I am pretty sure I am hurting your brain again :-)

bu5hm4n added inline comments.Sat, Mar 16, 11:06 AM
src/lib/eio/efl_io_model.c
455

Can you point me to this logic ? Because the places i found are infact checking for a promise not for a future.

cedric added inline comments.Sat, Mar 16, 11:20 AM
src/lib/eio/efl_io_model.c
455

Oh, that's because there is no EINA_VALUE_TYPE_FUTURE only EINA_VALUE_TYPE_PROMISE. If you look at the code of eina_future_as_value in eina_promise.c, you will see how things are hooked via an intermediate promise.

bu5hm4n accepted this revision.Sat, Mar 16, 11:35 AM

We might want to consider renaming eina_future_as_value to something that does *not* suggest that the eina_value is a future :D

This revision is now accepted and ready to land.Sat, Mar 16, 11:35 AM

We might want to consider renaming eina_future_as_value to something that does *not* suggest that the eina_value is a future :D

I don't know. It does take an Eina_Future and make it an Eina_Value. There is just some magic plumbing inside. I don't know if the user of the function has to know much about that magic.

We might want to consider renaming eina_future_as_value to something that does *not* suggest that the eina_value is a future :D

I don't know. It does take an Eina_Future and make it an Eina_Value. There is just some magic plumbing inside. I don't know if the user of the function has to know much about that magic.

Sure, but the function name indicates that the eina_value will be a future and not a promise, this is quite confusing to me. maybe @segfaultxavi has also an opinion on that ?

First off, thank you both for giving my brain the opportunity to hurt so much.

So, there is a method called eina_promise_as_value() which takes an Eina_Promise and wraps it in an Eina_Value.
And then there is eina_future_as_value() which indeed takes an Eina_Future and produces an Eina_Value. The mechanism, though, is obscure to me and I would appreciate an explanation. A proxy promise is created, and another callback is attached to the original future... but I get lost.

At any rate, eina_promise_as_value is very simple, whereas eina_future_as_value seems to be doing a lot of things. So yeah, until I understand better what is going on, I also think that "future as value" is misleading.

First off, thank you both for giving my brain the opportunity to hurt so much.

You are welcome :-)

The idea for propagating a future into an Eina_Value without duplicating logic and adding a new type is to actually create a promise and put that into the Eina_Value. Then setting up a callback on the Eina_Future that was given and when it resolve manually trigger a resolve on the promise that was set inside the Eina_Value. This is like a manual pipe to get things across. Of course if the Eina_Value is freed, then it will cancel the callback. I hope this help.

Understood, but eina_future_as_value() does not wrap an Eina_Future in the value, it wraps an Eina_Promise, so... misleading :)

Anyway, that has nothing to do with this patch, right? I created another task for this: T7761.