On 08/03/16 13:39, Tom Hacohen wrote:
> Hey,
>
> Wouldn't it better if the intercept function could return a value saying
> "actually, the object is good to go, just delete it normally", like in
> your example, if the object is already in the right thread, it should
> just delete immediately, no?

To be honest, giving it a bit more thought I don't see how that is 
useful. I think that it's an extreme corner case that is not worth the 
trouble, and I'm very much against adding this kind of API because we 
actually have uses for them. I'm talking about code using them. Adding 
this won't break API so we can wait with adding it until when we 
actually need it.

An alternative would be to deal with the cleanup in the destructor. So 
you for example free data that is thread agnostic, and when you have 
code that needs to be run in a specific loop, you can set manual free 
and defer the cleanup to that thread, so most of the object is 
destructed in the destructing thread, but the relevant parts are cleaned 
in the correct thread. This is implemented per object and is much cleaner.

I like it better, but even if we choose to go with the idea in this 
commit, I'd say: revert it until needed. I talked to Felipe about it on 
IRC and he agrees.

--
Tom.

>
> --
> Tom
>
> On 08/03/16 08:33, Carsten Haitzler wrote:
>> raster pushed a commit to branch master.
>>
>> http://git.enlightenment.org/core/efl.git/commit/?id=3df71ab0f668967cf37813cc2322d1993a4d5db1
>>
>> commit 3df71ab0f668967cf37813cc2322d1993a4d5db1
>> Author: Carsten Haitzler (Rasterman) <ras...@rasterman.com>
>> Date:   Tue Mar 8 16:57:22 2016 +0900.
>>
>>       eo del interceptor: add the ability to intercept deletions of eo 
>> objects
>>
>>       Imagine this. You have an object. You pass this object handle as a
>>       message to another thread. Let's say it's not a UI object, so
>>       something you might expect to be able to be accessed from multiple
>>       threads. In order to keep the object alive you eo_ref() it when
>>       placing the message on a queue and eo_unref() it once the message is
>>       "done" in the other thread. If the original sender unref()ed the
>>       object before the message is done, then the object will be destroyed
>>       in the reciever thread. This is bad for objects "expecting" not to be
>>       destroyed outside their owning thread.
>>
>>       This allows thius situation to be fixed. A constructor in a class of
>>       an object can set up a delete interceptor. For example if we have a
>>       "loop ownership" class you multi-ple-inherit from/use as a mixin. This
>>       class will set up the interceptor to ensure that on destruction if
>>       pthread_self() != owning loop thread id, then add object to "delete
>>       me" queue on the owning loop and wake it up. the owning loop thread
>>       will wake up and then process this queue and delete the queued objects
>>       nicely and safely within the "owning context".
>>
>>       This can also be used in this same manner to defer deletion within a
>>       loop "until later" in the same delete_me queue.
>>
>>       You can even use this as a caching mechanism for objects to prevernt
>>       their actual destruction and instead place them in a cached area to be
>>       picked from at a later date.
>>
>>       The uses are many for this and this is a basic building block for
>>       future EFL features like generic messages where a message payload
>>       could be an eo object and thus the above loop onwership issue can
>>       happen and needs fixing.
>>
>>       This adds APIs, implementation, documentation (doxy reference) and 
>> tests.
>>
>>       @feature
>> ---
>>    src/lib/eo/Eo.h                      | 58 
>> ++++++++++++++++++++++++++++++++++++
>>    src/lib/eo/eo.c                      | 16 ++++++++++
>>    src/lib/eo/eo_private.h              |  9 ++++++
>>    src/tests/eo/suite/eo_test_general.c | 55 
>> ++++++++++++++++++++++++++++++++++
>>    4 files changed, 138 insertions(+)
>>
>> diff --git a/src/lib/eo/Eo.h b/src/lib/eo/Eo.h
>> index cf10bb3..4bfae97 100644
>> --- a/src/lib/eo/Eo.h
>> +++ b/src/lib/eo/Eo.h
>> @@ -166,6 +166,16 @@ typedef struct _Eo_Event Eo_Event;
>>     */
>>    typedef Eina_Bool (*Eo_Event_Cb)(void *data, const Eo_Event *event);
>>
>> +/**
>> + * @typedef Eo_Del_Intercept
>> + *
>> + * A function to be called on object deletion/destruction instead of normal
>> + * destruction taking place.
>> + *
>> + * @param obj_id The object needing destruction
>> + */
>> +typedef void (*Eo_Del_Intercept) (Eo *obj_id);
>> +
>>    #include "eo_base.eo.h"
>>    #define EO_CLASS EO_BASE_CLASS
>>
>> @@ -787,6 +797,54 @@ EAPI void eo_unref(const Eo *obj);
>>    EAPI int eo_ref_get(const Eo *obj);
>>
>>    /**
>> + * @brief Set a deletion interceptor function
>> + * @param obj The object to set the interceptor on
>> + * @param del_intercept_func The interceptor function to call
>> + *
>> + * This sets the function @p del_intercept_func to be called when an object
>> + * is about to go from a reference count of 1 to 0, thus triggering actual
>> + * destruction of the object. Instead of going to a reference count of 0 and
>> + * being destroyed, the object will stay alive with a reference count of 1
>> + * and this intercept function will be called instead. It is the job of
>> + * this interceptor function to handle any further deletion of of the object
>> + * from here.
>> + *
>> + * Note that by default objects have no interceptor function set, and thus
>> + * will be destroyed as normal. To return an object to this state, simply
>> + * set the @p del_intercept_func to NULL which is the default.
>> + *
>> + * A good use for this feature is to ensure an object is destroyed by its
>> + * owning main loop and not in a foreign loop. This makes it possible to
>> + * safely unrefor delete objects from any loop as an interceptor can be set
>> + * on an object that will abort destruction and instead queue the object
>> + * on its owning loop to be destroyed at some time in the future and now
>> + * set the intercept function to NULL so it is not called again on the next
>> + * "real deletion".
>> + *
>> + * @see eo_del_intercept_get()
>> + * @see eo_unref()
>> + * @see eo_del()
>> + */
>> +EAPI void eo_del_intercept_set(Eo *obj, Eo_Del_Intercept 
>> del_intercept_func);
>> +
>> +/**
>> + * @brief Get the deletion interceptor function
>> + * @param obj The object to get the interceptor of
>> + * @return The intercept function or NULL if none is set.
>> + *
>> + * This returns the interceptor function set by eo_del_intercept_set(). Note
>> + * that objects by default have no interceptor (NULL) set, but certain
>> + * classes may set one up in a constructor, so it is important to be able
>> + * to get the interceptor function to know if this has happend and
>> + * if you want to override this interceptor, be sure to call it after your
>> + * own interceptor function has finished. It would generally be a bad idea
>> + * though to override these functions.
>> + *
>> + * @see eo_del_intercept_set()
>> + */
>> +EAPI Eo_Del_Intercept eo_del_intercept_get(const Eo *obj);
>> +
>> +/**
>>     * @brief Unrefs the object and reparents it to NULL.
>>     * @param obj the object to work on.
>>     *
>> diff --git a/src/lib/eo/eo.c b/src/lib/eo/eo.c
>> index 996b961..ba12fbd 100644
>> --- a/src/lib/eo/eo.c
>> +++ b/src/lib/eo/eo.c
>> @@ -1332,6 +1332,22 @@ eo_ref_get(const Eo *obj_id)
>>       return obj->refcount;
>>    }
>>
>> +EAPI void
>> +eo_del_intercept_set(Eo *obj_id, Eo_Del_Intercept del_intercept_func)
>> +{
>> +   EO_OBJ_POINTER_RETURN(obj_id, obj);
>> +
>> +   obj->del_intercept = del_intercept_func;
>> +}
>> +
>> +EAPI Eo_Del_Intercept
>> +eo_del_intercept_get(const Eo *obj_id)
>> +{
>> +   EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, NULL);
>> +
>> +   return obj->del_intercept;
>> +}
>> +
>>    void
>>    _eo_condtor_done(Eo *obj_id)
>>    {
>> diff --git a/src/lib/eo/eo_private.h b/src/lib/eo/eo_private.h
>> index 3ee4134..323fc02 100644
>> --- a/src/lib/eo/eo_private.h
>> +++ b/src/lib/eo/eo_private.h
>> @@ -95,6 +95,7 @@ struct _Eo_Object
>>    #endif
>>
>>         Eina_List *composite_objects;
>> +     Eo_Del_Intercept del_intercept;
>>
>>         int refcount;
>>         int datarefcount;
>> @@ -295,6 +296,14 @@ _eo_unref(_Eo_Object *obj)
>>                 ERR("Object %p deletion already triggered. You wrongly call 
>> eo_unref() within a destructor.", _eo_id_get(obj));
>>                 return;
>>              }
>> +
>> +        if (obj->del_intercept)
>> +          {
>> +             (obj->refcount)++;
>> +             obj->del_intercept(_eo_id_get(obj));
>> +             return;
>> +          }
>> +
>>            obj->del_triggered = EINA_TRUE;
>>
>>            _eo_del_internal(__FILE__, __LINE__, obj);
>> diff --git a/src/tests/eo/suite/eo_test_general.c 
>> b/src/tests/eo/suite/eo_test_general.c
>> index 9a0db0f..2960902 100644
>> --- a/src/tests/eo/suite/eo_test_general.c
>> +++ b/src/tests/eo/suite/eo_test_general.c
>> @@ -950,6 +950,60 @@ START_TEST(eo_add_failures)
>>    }
>>    END_TEST
>>
>> +static Eina_Bool intercepted = EINA_FALSE;
>> +
>> +static void
>> +_del_intercept(Eo *obj)
>> +{
>> +   intercepted = EINA_TRUE;
>> +   eo_del_intercept_set(obj, NULL);
>> +   eo_unref(obj);
>> +}
>> +
>> +START_TEST(eo_del_intercept)
>> +{
>> +#ifdef HAVE_EO_ID
>> +   eo_init();
>> +
>> +   static const Eo_Class_Description class_desc = {
>> +        EO_VERSION,
>> +        "Simple",
>> +        EO_CLASS_TYPE_REGULAR,
>> +        EO_CLASS_DESCRIPTION_NOOPS(),
>> +        NULL,
>> +        0,
>> +        NULL,
>> +        NULL
>> +   };
>> +
>> +   const Eo_Class *klass = eo_class_new(&class_desc, EO_CLASS, NULL);
>> +   fail_if(!klass);
>> +
>> +   /* Check unref interception */
>> +   intercepted = EINA_FALSE;
>> +   Eo *obj = eo_add(klass, NULL);
>> +   fail_if(!obj);
>> +   fail_if(!eo_isa(obj, klass));
>> +   eo_del_intercept_set(obj, _del_intercept);
>> +   eo_unref(obj);
>> +   fail_if(!intercepted);
>> +   fail_if(eo_isa(obj, klass));
>> +
>> +   /* Check del interception */
>> +   intercepted = EINA_FALSE;
>> +   obj = eo_add(klass, NULL);
>> +   fail_if(!obj);
>> +   fail_if(!eo_isa(obj, klass));
>> +   eo_del_intercept_set(obj, _del_intercept);
>> +   eo_del(obj);
>> +   fail_if(!intercepted);
>> +   fail_if(eo_isa(obj, klass));
>> +
>> +   eo_shutdown();
>> +#endif
>> +}
>> +END_TEST
>> +
>>    void eo_test_general(TCase *tc)
>>    {
>>       tcase_add_test(tc, eo_simple);
>> @@ -967,4 +1021,5 @@ void eo_test_general(TCase *tc)
>>       tcase_add_test(tc, eo_add_do_and_custom);
>>       tcase_add_test(tc, eo_pointers_indirection);
>>       tcase_add_test(tc, eo_add_failures);
>> +   tcase_add_test(tc, eo_del_intercept);
>>    }
>>
>
>
> ------------------------------------------------------------------------------
> Transform Data into Opportunity.
> Accelerate data analysis in your applications with
> Intel Data Analytics Acceleration Library.
> Click to learn more.
> http://makebettercode.com/inteldaal-eval
> _______________________________________________
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>


------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785111&iu=/4140
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to