eo id and shared domain objects - do locking properly and better

so there were a few issues. one we had a spinlokc on the eoid table
for shared objects AND then had a mutex for accessing those objects
(released on return from any eo function). BUT this missed some funcs
like eo_ref, eo_unref and so on in eo.c ... oops. so fixed. but then i
realized there was a race condition. we locked the eoid table then
unlocked with our pointer THEN locked the sharted object mutex ...
then unlocked it. that was a race condtion gap. so we should share the
same lock anyway - if it's a shared object, grab the shared object
mutex then do a lookup and if the lookup does not fail, KEEP the lock
until it is released by the return from eo function or by some special
macro/funcs that released a matching lock. since its a recursive lock
this is all fine. as its also a universal single lock for all objects
we just need the eoid to know if it's shared and needs locking based
on the domain bits. so now do this locking properly with just a single
mutex, not both a spinlock and mutex and keep the lock around until
totally done with the object. this plugs the race condition holes and
goes from 1 spinlock lock and unlock then a mutex lock and unlokc to
just a single mutex lock and unlock. this means shared objects are
actually truly safe across threads and only have the overhead of a
single recursive mutex to lock and unlock in every api call.
This commit is contained in:
Carsten Haitzler 2016-09-28 13:25:26 +09:00
parent 0cbb5a42f3
commit 0f929f5546
7 changed files with 218 additions and 155 deletions

View File

@ -691,7 +691,6 @@ typedef struct _Efl_Object_Op_Call_Data
_Eo_Object *obj; _Eo_Object *obj;
void *func; void *func;
void *data; void *data;
void *lock_data;
void *extn1; // for the future to avoid ABI issues void *extn1; // for the future to avoid ABI issues
void *extn2; // for the future to avoid ABI issues void *extn2; // for the future to avoid ABI issues
void *extn3; // for the future to avoid ABI issues void *extn3; // for the future to avoid ABI issues

View File

@ -335,8 +335,6 @@ _efl_object_call_resolve(Eo *eo_id, const char *func_name, Efl_Object_Op_Call_Da
const op_type_funcs *func; const op_type_funcs *func;
Eina_Bool is_obj; Eina_Bool is_obj;
Eina_Bool is_override = EINA_FALSE; Eina_Bool is_override = EINA_FALSE;
Eo_Id_Data *data;
Efl_Id_Domain domain = 0;
if (((Eo_Id) eo_id) & MASK_SUPER_TAG) if (((Eo_Id) eo_id) & MASK_SUPER_TAG)
{ {
@ -351,14 +349,12 @@ _efl_object_call_resolve(Eo *eo_id, const char *func_name, Efl_Object_Op_Call_Da
return EINA_FALSE; return EINA_FALSE;
call->eo_id = eo_id; call->eo_id = eo_id;
call->lock_data = NULL;
is_obj = _eo_is_a_obj(eo_id); is_obj = _eo_is_a_obj(eo_id);
if (is_obj) if (is_obj)
{ {
EO_OBJ_POINTER_RETURN_VAL(eo_id, _obj, EINA_FALSE); EO_OBJ_POINTER_RETURN_VAL(eo_id, _obj, EINA_FALSE);
domain = ((Eo_Id)eo_id >> SHIFT_DOMAIN) & MASK_DOMAIN;
obj = _obj; obj = _obj;
klass = _obj->klass; klass = _obj->klass;
@ -376,14 +372,6 @@ _efl_object_call_resolve(Eo *eo_id, const char *func_name, Efl_Object_Op_Call_Da
is_override = _obj_is_override(obj) && (cur_klass == NULL); is_override = _obj_is_override(obj) && (cur_klass == NULL);
call->obj = obj; call->obj = obj;
if (EINA_UNLIKELY(domain == EFL_ID_DOMAIN_SHARED))
{
// YES this is a goto with a label to return. this is a
// micro-optimization to move infrequent code out of the
// hot path of the function
goto ok_lock;
}
ok_lock_back:
_efl_ref(_obj); _efl_ref(_obj);
} }
else else
@ -515,8 +503,7 @@ err_func_src:
ERR("in %s:%d: you called a pure virtual func '%s' (%d) of class '%s'.", ERR("in %s:%d: you called a pure virtual func '%s' (%d) of class '%s'.",
file, line, func_name, cache->op, klass->desc->name); file, line, func_name, cache->op, klass->desc->name);
err: err:
if (EINA_LIKELY(domain != EFL_ID_DOMAIN_SHARED)) return EINA_FALSE; if (is_obj) _eo_obj_pointer_done((Eo_Id)eo_id);
eina_lock_take(&(data->tables[EFL_ID_DOMAIN_SHARED]->obj_lock));
return EINA_FALSE; return EINA_FALSE;
// yes - special "move out of hot path" code blobs with goto's for // yes - special "move out of hot path" code blobs with goto's for
@ -539,13 +526,6 @@ ok_klass:
} }
goto ok_klass_back; goto ok_klass_back;
ok_lock:
{
data = call->lock_data = _eo_table_data_get();
eina_lock_take(&(data->tables[EFL_ID_DOMAIN_SHARED]->obj_lock));
}
goto ok_lock_back;
return EINA_FALSE; return EINA_FALSE;
} }
@ -555,13 +535,32 @@ _efl_object_call_end(Efl_Object_Op_Call_Data *call)
if (EINA_LIKELY(!!call->obj)) if (EINA_LIKELY(!!call->obj))
{ {
_efl_unref(call->obj); _efl_unref(call->obj);
if (EINA_LIKELY(!call->lock_data)) return; if (call->obj) _eo_obj_pointer_done((Eo_Id)call->eo_id);
Eo_Id_Data *data = call->lock_data;
eina_lock_release(&(data->tables[EFL_ID_DOMAIN_SHARED]->obj_lock));
} }
} }
/*
EAPI void
_efl_shared_lock(const Eo *eo_id)
{
#ifdef HAVE_EO_ID
Efl_Id_Domain domain = ((Eo_Id)eo_id >> SHIFT_DOMAIN) & MASK_DOMAIN;
if (EINA_LIKELY(domain != EFL_ID_DOMAIN_SHARED)) return;
eina_lock_take(&(_eo_table_data_shared_data->obj_lock));
#endif
}
EAPI void
_efl_shared_unlock(const Eo *eo_id)
{
#ifdef HAVE_EO_ID
Efl_Id_Domain domain = ((Eo_Id)eo_id >> SHIFT_DOMAIN) & MASK_DOMAIN;
if (EINA_LIKELY(domain != EFL_ID_DOMAIN_SHARED)) return;
eina_lock_release(&(_eo_table_data_shared_data->obj_lock));
#endif
}
*/
static inline Eina_Bool static inline Eina_Bool
_eo_api_func_equal(const void *api_func1, const void *api_func2) _eo_api_func_equal(const void *api_func1, const void *api_func2)
{ {
@ -748,6 +747,7 @@ _efl_add_internal_start(const char *file, int line, const Efl_Class *klass_id, E
if (EINA_UNLIKELY(klass->desc->type != EFL_CLASS_TYPE_REGULAR)) if (EINA_UNLIKELY(klass->desc->type != EFL_CLASS_TYPE_REGULAR))
{ {
ERR("in %s:%d: Class '%s' is not instantiate-able. Aborting.", file, line, klass->desc->name); ERR("in %s:%d: Class '%s' is not instantiate-able. Aborting.", file, line, klass->desc->name);
if (parent_id) EO_OBJ_DONE(parent_id);
return NULL; return NULL;
} }
@ -791,6 +791,7 @@ _efl_add_internal_start(const char *file, int line, const Efl_Class *klass_id, E
/* We have two refs at this point. */ /* We have two refs at this point. */
_efl_unref(obj); _efl_unref(obj);
efl_del((Eo *) obj->header.id); efl_del((Eo *) obj->header.id);
if (parent_id) EO_OBJ_DONE(parent_id);
return NULL; return NULL;
} }
else if (eo_id != _eo_obj_id_get(obj)) else if (eo_id != _eo_obj_id_get(obj))
@ -801,6 +802,7 @@ _efl_add_internal_start(const char *file, int line, const Efl_Class *klass_id, E
efl_del((Eo *) obj->header.id); efl_del((Eo *) obj->header.id);
_efl_ref(new_obj); _efl_ref(new_obj);
EO_OBJ_DONE(eo_id);
} }
if (is_fallback) if (is_fallback)
@ -808,6 +810,7 @@ _efl_add_internal_start(const char *file, int line, const Efl_Class *klass_id, E
fptr->obj = eo_id; fptr->obj = eo_id;
} }
if (parent_id) EO_OBJ_DONE(parent_id);
return eo_id; return eo_id;
} }
@ -844,12 +847,13 @@ _efl_add_internal_end(Eo *eo_id, Eo *finalized_id)
obj->finalized = EINA_TRUE; obj->finalized = EINA_TRUE;
_efl_unref(obj); _efl_unref(obj);
EO_OBJ_DONE(eo_id);
return (Eo *)eo_id; return (Eo *)eo_id;
cleanup: cleanup:
_efl_unref(obj); _efl_unref(obj);
efl_del((Eo *) obj->header.id); efl_del((Eo *) obj->header.id);
EO_OBJ_DONE(eo_id);
return NULL; return NULL;
} }
@ -877,6 +881,8 @@ _efl_add_end(Eo *eo_id, Eina_Bool is_ref, Eina_Bool is_fallback)
EAPI const Efl_Class * EAPI const Efl_Class *
efl_class_get(const Eo *eo_id) efl_class_get(const Eo *eo_id)
{ {
const Efl_Class *klass;
if (_eo_is_a_class(eo_id)) if (_eo_is_a_class(eo_id))
{ {
EO_CLASS_POINTER_RETURN_VAL(eo_id, _klass, NULL); EO_CLASS_POINTER_RETURN_VAL(eo_id, _klass, NULL);
@ -884,8 +890,9 @@ efl_class_get(const Eo *eo_id)
} }
EO_OBJ_POINTER_RETURN_VAL(eo_id, obj, NULL); EO_OBJ_POINTER_RETURN_VAL(eo_id, obj, NULL);
klass = _eo_class_id_get(obj->klass);
return _eo_class_id_get(obj->klass); EO_OBJ_DONE(eo_id);
return klass;
} }
EAPI const char * EAPI const char *
@ -902,8 +909,8 @@ efl_class_name_get(const Efl_Class *eo_id)
{ {
EO_OBJ_POINTER_RETURN_VAL(eo_id, obj, NULL); EO_OBJ_POINTER_RETURN_VAL(eo_id, obj, NULL);
klass = obj->klass; klass = obj->klass;
EO_OBJ_DONE(eo_id);
} }
return klass->desc->name; return klass->desc->name;
} }
@ -1403,36 +1410,41 @@ efl_object_override(Eo *eo_id, const Efl_Object_Ops *ops)
EO_CLASS_POINTER_RETURN_VAL(EFL_OBJECT_OVERRIDE_CLASS, klass, EINA_FALSE); EO_CLASS_POINTER_RETURN_VAL(EFL_OBJECT_OVERRIDE_CLASS, klass, EINA_FALSE);
Eo_Vtable *previous = obj->vtable; Eo_Vtable *previous = obj->vtable;
if (!ops) if (ops)
{
if (obj->vtable == &obj->klass->vtable)
{
obj->vtable = calloc(1, sizeof(*obj->vtable));
_vtable_init(obj->vtable, previous->size);
_vtable_copy_all(obj->vtable, previous);
if (!_eo_class_funcs_set(obj->vtable, ops, obj->klass,
klass, 0, EINA_TRUE))
goto err;
goto done;
}
else goto err_already;
}
else
{ {
if (obj->vtable != &obj->klass->vtable) if (obj->vtable != &obj->klass->vtable)
{ {
free(obj->vtable); free(obj->vtable);
obj->vtable = (Eo_Vtable *) &obj->klass->vtable; obj->vtable = (Eo_Vtable *) &obj->klass->vtable;
} }
}
done:
EO_OBJ_DONE(eo_id);
return EINA_TRUE; return EINA_TRUE;
}
if (obj->vtable == &obj->klass->vtable) err_already:
{
obj->vtable = calloc(1, sizeof(*obj->vtable));
_vtable_init(obj->vtable, previous->size);
_vtable_copy_all(obj->vtable, previous);
}
else
{
ERR("Function table already overridden, not allowed to override again. " ERR("Function table already overridden, not allowed to override again. "
"Call with NULL to reset the function table first."); "Call with NULL to reset the function table first.");
return EINA_FALSE; goto err_done;
} err:
if (!_eo_class_funcs_set(obj->vtable, ops, obj->klass, klass, 0, EINA_TRUE))
{
ERR("Failed to override functions for %p", eo_id); ERR("Failed to override functions for %p", eo_id);
err_done:
EO_OBJ_DONE(eo_id);
return EINA_FALSE; return EINA_FALSE;
}
return EINA_TRUE;
} }
EAPI Eina_Bool EAPI Eina_Bool
@ -1471,7 +1483,7 @@ efl_isa(const Eo *eo_id, const Efl_Class *klass_id)
} }
else else
{ {
eina_spinlock_take(&(tdata->lock)); eina_lock_take(&(_eo_table_data_shared_data->obj_lock));
if ((tdata->cache.isa_id == eo_id) && if ((tdata->cache.isa_id == eo_id) &&
(tdata->cache.klass == klass_id)) (tdata->cache.klass == klass_id))
@ -1479,15 +1491,12 @@ efl_isa(const Eo *eo_id, const Efl_Class *klass_id)
isa = tdata->cache.isa; isa = tdata->cache.isa;
goto shared_ok; goto shared_ok;
} }
eina_spinlock_release(&(tdata->lock));
EO_OBJ_POINTER_RETURN_VAL(eo_id, obj, EINA_FALSE); EO_OBJ_POINTER_RETURN_VAL(eo_id, obj, EINA_FALSE);
EO_CLASS_POINTER_RETURN_VAL(klass_id, klass, EINA_FALSE); EO_CLASS_POINTER_RETURN_VAL(klass_id, klass, EINA_FALSE);
const op_type_funcs *func = _vtable_func_get const op_type_funcs *func = _vtable_func_get
(obj->vtable, klass->base_id + klass->ops_count); (obj->vtable, klass->base_id + klass->ops_count);
eina_spinlock_take(&(tdata->lock));
// Caching the result as we do a lot of serial efl_isa due to evas_object_image using it. // Caching the result as we do a lot of serial efl_isa due to evas_object_image using it.
tdata->cache.isa_id = eo_id; tdata->cache.isa_id = eo_id;
tdata->cache.klass = klass_id; tdata->cache.klass = klass_id;
@ -1495,7 +1504,7 @@ efl_isa(const Eo *eo_id, const Efl_Class *klass_id)
// _eo_class_isa_func. // _eo_class_isa_func.
isa = tdata->cache.isa = (func && (func->func == _eo_class_isa_func)); isa = tdata->cache.isa = (func && (func->func == _eo_class_isa_func));
shared_ok: shared_ok:
eina_spinlock_release(&(tdata->lock)); eina_lock_release(&(_eo_table_data_shared_data->obj_lock));
} }
return isa; return isa;
} }
@ -1514,6 +1523,7 @@ efl_xref_internal(const char *file, int line, Eo *obj_id, const Eo *ref_obj_id)
xref->line = line; xref->line = line;
obj->xrefs = eina_inlist_prepend(obj->xrefs, EINA_INLIST_GET(xref)); obj->xrefs = eina_inlist_prepend(obj->xrefs, EINA_INLIST_GET(xref));
EO_OBJ_DONE(obj_id);
#else #else
(void) ref_obj_id; (void) ref_obj_id;
(void) file; (void) file;
@ -1543,8 +1553,10 @@ efl_xunref(Eo *obj_id, const Eo *ref_obj_id)
else else
{ {
ERR("ref_obj (%p) does not reference obj (%p). Aborting unref.", ref_obj_id, obj_id); ERR("ref_obj (%p) does not reference obj (%p). Aborting unref.", ref_obj_id, obj_id);
EO_OBJ_DONE(obj_id);
return; return;
} }
EO_OBJ_DONE(obj_id);
#else #else
(void) ref_obj_id; (void) ref_obj_id;
#endif #endif
@ -1558,9 +1570,8 @@ efl_ref(const Eo *obj_id)
++(obj->user_refcount); ++(obj->user_refcount);
if (EINA_UNLIKELY(obj->user_refcount == 1)) if (EINA_UNLIKELY(obj->user_refcount == 1))
{
_efl_ref(obj); _efl_ref(obj);
} EO_OBJ_DONE(obj_id);
return (Eo *)obj_id; return (Eo *)obj_id;
} }
@ -1575,27 +1586,32 @@ efl_unref(const Eo *obj_id)
if (obj->user_refcount < 0) if (obj->user_refcount < 0)
{ {
ERR("Obj:%p. User refcount (%d) < 0. Too many unrefs.", obj, obj->user_refcount); ERR("Obj:%p. User refcount (%d) < 0. Too many unrefs.", obj, obj->user_refcount);
EO_OBJ_DONE(obj_id);
return; return;
} }
_efl_unref(obj); _efl_unref(obj);
} }
EO_OBJ_DONE(obj_id);
} }
EAPI int EAPI int
efl_ref_get(const Eo *obj_id) efl_ref_get(const Eo *obj_id)
{ {
EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, 0); EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, 0);
int ref;
return obj->user_refcount; ref = obj->user_refcount;
EO_OBJ_DONE(obj_id);
return ref;
} }
EAPI int EAPI int
___efl_ref2_get(const Eo *obj_id) ___efl_ref2_get(const Eo *obj_id)
{ {
EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, 0); EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, 0);
int ref;
return obj->refcount; ref = obj->refcount;
EO_OBJ_DONE(obj_id);
return ref;
} }
EAPI void EAPI void
@ -1603,6 +1619,7 @@ ___efl_ref2_reset(const Eo *obj_id)
{ {
EO_OBJ_POINTER_RETURN(obj_id, obj); EO_OBJ_POINTER_RETURN(obj_id, obj);
obj->refcount = 0; obj->refcount = 0;
EO_OBJ_DONE(obj_id);
} }
@ -1610,16 +1627,18 @@ EAPI void
efl_del_intercept_set(Eo *obj_id, Efl_Del_Intercept del_intercept_func) efl_del_intercept_set(Eo *obj_id, Efl_Del_Intercept del_intercept_func)
{ {
EO_OBJ_POINTER_RETURN(obj_id, obj); EO_OBJ_POINTER_RETURN(obj_id, obj);
obj->del_intercept = del_intercept_func; obj->del_intercept = del_intercept_func;
EO_OBJ_DONE(obj_id);
} }
EAPI Efl_Del_Intercept EAPI Efl_Del_Intercept
efl_del_intercept_get(const Eo *obj_id) efl_del_intercept_get(const Eo *obj_id)
{ {
EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, NULL); EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, NULL);
Efl_Del_Intercept func;
return obj->del_intercept; func = obj->del_intercept;
EO_OBJ_DONE(obj_id);
return func;
} }
void void
@ -1629,10 +1648,11 @@ _eo_condtor_done(Eo *obj_id)
if (obj->condtor_done) if (obj->condtor_done)
{ {
ERR("Object %p is already constructed at this point.", obj); ERR("Object %p is already constructed at this point.", obj);
EO_OBJ_DONE(obj_id);
return; return;
} }
obj->condtor_done = EINA_TRUE; obj->condtor_done = EINA_TRUE;
EO_OBJ_DONE(obj_id);
} }
static inline void * static inline void *
@ -1755,52 +1775,52 @@ efl_data_scope_get(const Eo *obj_id, const Efl_Class *klass_id)
if (!_eo_class_mro_has(obj->klass, klass)) if (!_eo_class_mro_has(obj->klass, klass))
{ {
ERR("Tried getting data of class '%s' from object of class '%s', but the former is not a direct inheritance of the latter.", klass->desc->name, obj->klass->desc->name); ERR("Tried getting data of class '%s' from object of class '%s', but the former is not a direct inheritance of the latter.", klass->desc->name, obj->klass->desc->name);
EO_OBJ_DONE(obj_id);
return NULL; return NULL;
} }
#endif #endif
ret = _efl_data_scope_safe_get(obj, klass); ret = _efl_data_scope_safe_get(obj, klass);
#ifdef EO_DEBUG #ifdef EO_DEBUG
if (!ret && (klass->desc->data_size == 0)) if (!ret && (klass->desc->data_size == 0))
{
ERR("Tried getting data of class '%s', but it has none.", klass->desc->name); ERR("Tried getting data of class '%s', but it has none.", klass->desc->name);
}
#endif #endif
EO_OBJ_DONE(obj_id);
return ret; return ret;
} }
EAPI void * EAPI void *
efl_data_xref_internal(const char *file, int line, const Eo *obj_id, const Efl_Class *klass_id, const Eo *ref_obj_id) efl_data_xref_internal(const char *file, int line, const Eo *obj_id, const Efl_Class *klass_id, const Eo *ref_obj_id)
{ {
void *ret; void *ret = NULL;
EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, NULL);
EO_OBJ_POINTER_RETURN_VAL(ref_obj_id, ref_obj, NULL);
_Efl_Class *klass = NULL; _Efl_Class *klass = NULL;
EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, NULL);
EO_OBJ_POINTER(ref_obj_id, ref_obj);
if (ref_obj)
{
if (klass_id) if (klass_id)
{ {
EO_CLASS_POINTER_RETURN_VAL(klass_id, klass2, NULL); EO_CLASS_POINTER_RETURN_VAL(klass_id, klass2, NULL);
klass = klass2; klass = klass2;
#ifdef EO_DEBUG #ifdef EO_DEBUG
if (!_eo_class_mro_has(obj->klass, klass)) if (!_eo_class_mro_has(obj->klass, klass))
{ {
ERR("Tried getting data of class '%s' from object of class '%s', but the former is not a direct inheritance of the latter.", klass->desc->name, obj->klass->desc->name); ERR("Tried getting data of class '%s' from object of class '%s', but the former is not a direct inheritance of the latter.", klass->desc->name, obj->klass->desc->name);
EO_OBJ_DONE(obj_id);
EO_OBJ_DONE(ref_obj_id);
return NULL; return NULL;
} }
#endif #endif
} }
ret = _efl_data_xref_internal(file, line, obj, klass, ref_obj); ret = _efl_data_xref_internal(file, line, obj, klass, ref_obj);
#ifdef EO_DEBUG #ifdef EO_DEBUG
if (klass && !ret && (klass->desc->data_size == 0)) if (klass && !ret && (klass->desc->data_size == 0))
{
ERR("Tried getting data of class '%s', but it has none.", klass->desc->name); ERR("Tried getting data of class '%s', but it has none.", klass->desc->name);
}
#endif #endif
EO_OBJ_DONE(ref_obj_id);
}
EO_OBJ_DONE(obj_id);
return ret; return ret;
} }
@ -1808,8 +1828,13 @@ EAPI void
efl_data_xunref_internal(const Eo *obj_id, void *data, const Eo *ref_obj_id) efl_data_xunref_internal(const Eo *obj_id, void *data, const Eo *ref_obj_id)
{ {
EO_OBJ_POINTER_RETURN(obj_id, obj); EO_OBJ_POINTER_RETURN(obj_id, obj);
EO_OBJ_POINTER_RETURN(ref_obj_id, ref_obj); EO_OBJ_POINTER(ref_obj_id, ref_obj);
if (ref_obj)
{
_efl_data_xunref_internal(obj, data, ref_obj); _efl_data_xunref_internal(obj, data, ref_obj);
EO_OBJ_DONE(ref_obj_id);
}
EO_OBJ_DONE(obj_id);
} }
static void static void
@ -1880,6 +1905,7 @@ efl_object_init(void)
EINA_LOG_ERR("Could not allocate shared table data"); EINA_LOG_ERR("Could not allocate shared table data");
return EINA_FALSE; return EINA_FALSE;
} }
_eo_table_data_shared_data = _eo_table_data_shared->tables[EFL_ID_DOMAIN_SHARED];
// specially force eoid data to be creanted so we can switch it to domain 0 // specially force eoid data to be creanted so we can switch it to domain 0
Eo_Id_Data *data = _eo_table_data_new(EFL_ID_DOMAIN_MAIN); Eo_Id_Data *data = _eo_table_data_new(EFL_ID_DOMAIN_MAIN);
@ -1953,6 +1979,7 @@ efl_object_shutdown(void)
{ {
_eo_free_ids_tables(_eo_table_data_shared); _eo_free_ids_tables(_eo_table_data_shared);
_eo_table_data_shared = NULL; _eo_table_data_shared = NULL;
_eo_table_data_shared_data = NULL;
} }
eina_log_domain_unregister(_eo_log_dom); eina_log_domain_unregister(_eo_log_dom);
@ -2117,9 +2144,11 @@ efl_compatible(const Eo *obj, const Eo *obj_target)
EAPI Eina_Bool EAPI Eina_Bool
efl_destructed_is(const Eo *obj_id) efl_destructed_is(const Eo *obj_id)
{ {
Eina_Bool is;
EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, EINA_FALSE); EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, EINA_FALSE);
is = obj->destructed;
return obj->destructed; EO_OBJ_DONE(obj_id);
return is;
} }
EAPI void EAPI void
@ -2127,6 +2156,7 @@ efl_manual_free_set(Eo *obj_id, Eina_Bool manual_free)
{ {
EO_OBJ_POINTER_RETURN(obj_id, obj); EO_OBJ_POINTER_RETURN(obj_id, obj);
obj->manual_free = manual_free; obj->manual_free = manual_free;
EO_OBJ_DONE(obj_id);
} }
EAPI Eina_Bool EAPI Eina_Bool
@ -2134,21 +2164,21 @@ efl_manual_free(Eo *obj_id)
{ {
EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, EINA_FALSE); EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, EINA_FALSE);
if (EINA_FALSE == obj->manual_free) if (obj->manual_free == EINA_FALSE) goto err_manual_free;
{ if (!obj->destructed) goto err_not_destructed;
ERR("Tried to manually free the object %p while the option has not been set; see efl_manual_free_set for more information.", obj);
return EINA_FALSE;
}
if (!obj->destructed)
{
ERR("Tried deleting the object %p while still referenced(%d).", obj_id, obj->refcount);
return EINA_FALSE;
}
_eo_free(obj); _eo_free(obj);
EO_OBJ_DONE(obj_id);
return EINA_TRUE; return EINA_TRUE;
err_manual_free:
ERR("Tried to manually free the object %p while the option has not been set; see efl_manual_free_set for more information.", obj);
goto err;
err_not_destructed:
ERR("Tried deleting the object %p while still referenced(%d).", obj_id, obj->refcount);
goto err;
err:
EO_OBJ_DONE(obj_id);
return EINA_FALSE;
} }
EAPI int EAPI int

View File

@ -592,6 +592,7 @@ _efl_object_parent_set(Eo *obj, Efl_Object_Data *pd, Eo *parent_id)
{ {
pd->parent = NULL; pd->parent = NULL;
} }
EO_OBJ_DONE(obj);
} }
EOLIAN static Eo * EOLIAN static Eo *
@ -603,9 +604,11 @@ _efl_object_parent_get(Eo *obj EINA_UNUSED, Efl_Object_Data *pd)
EOLIAN static Eina_Bool EOLIAN static Eina_Bool
_efl_object_finalized_get(Eo *obj_id, Efl_Object_Data *pd EINA_UNUSED) _efl_object_finalized_get(Eo *obj_id, Efl_Object_Data *pd EINA_UNUSED)
{ {
Eina_Bool finalized;
EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, EINA_FALSE); EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, EINA_FALSE);
finalized = obj->finalized;
return obj->finalized; EO_OBJ_DONE(obj);
return finalized;
} }
EOLIAN static Efl_Object * EOLIAN static Efl_Object *
@ -679,7 +682,11 @@ _efl_object_children_iterator_new(Eo *obj_id, Efl_Object_Data *pd)
EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, NULL); EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, NULL);
if (!pd->children) return NULL; if (!pd->children)
{
EO_OBJ_DONE(obj_id);
return NULL;
}
klass = (_Efl_Class *) obj->klass; klass = (_Efl_Class *) obj->klass;
@ -706,6 +713,7 @@ _efl_object_children_iterator_new(Eo *obj_id, Efl_Object_Data *pd)
it->iterator.get_container = FUNC_ITERATOR_GET_CONTAINER(_efl_children_iterator_container); it->iterator.get_container = FUNC_ITERATOR_GET_CONTAINER(_efl_children_iterator_container);
it->iterator.free = FUNC_ITERATOR_FREE(_efl_children_iterator_free); it->iterator.free = FUNC_ITERATOR_FREE(_efl_children_iterator_free);
EO_OBJ_DONE(obj_id);
return (Eina_Iterator *)it; return (Eina_Iterator *)it;
} }
@ -1443,8 +1451,12 @@ EOLIAN static Eina_Bool
_efl_object_composite_attach(Eo *parent_id, Efl_Object_Data *pd EINA_UNUSED, Eo *comp_obj_id) _efl_object_composite_attach(Eo *parent_id, Efl_Object_Data *pd EINA_UNUSED, Eo *comp_obj_id)
{ {
EO_OBJ_POINTER_RETURN_VAL(comp_obj_id, comp_obj, EINA_FALSE); EO_OBJ_POINTER_RETURN_VAL(comp_obj_id, comp_obj, EINA_FALSE);
EO_OBJ_POINTER_RETURN_VAL(parent_id, parent, EINA_FALSE); EO_OBJ_POINTER(parent_id, parent);
if (!parent)
{
EO_OBJ_DONE(comp_obj_id);
return EINA_FALSE;
}
Efl_Object_Data *comp_pd = efl_data_scope_get(comp_obj_id, EFL_OBJECT_CLASS); Efl_Object_Data *comp_pd = efl_data_scope_get(comp_obj_id, EFL_OBJECT_CLASS);
/* Don't composite if we already have a composite object of this type */ /* Don't composite if we already have a composite object of this type */
{ {
@ -1454,14 +1466,16 @@ _efl_object_composite_attach(Eo *parent_id, Efl_Object_Data *pd EINA_UNUSED, Eo
{ {
EO_OBJ_POINTER_RETURN_VAL(emb_obj_id, emb_obj, EINA_FALSE); EO_OBJ_POINTER_RETURN_VAL(emb_obj_id, emb_obj, EINA_FALSE);
if (emb_obj->klass == comp_obj->klass) if (emb_obj->klass == comp_obj->klass)
{
EO_OBJ_DONE(parent_id);
EO_OBJ_DONE(comp_obj_id);
return EINA_FALSE; return EINA_FALSE;
} }
} }
}
if (efl_composite_part_is(comp_obj_id)) if (efl_composite_part_is(comp_obj_id))
{
efl_composite_detach(comp_pd->ext->composite_parent, comp_obj_id); efl_composite_detach(comp_pd->ext->composite_parent, comp_obj_id);
}
/* Set the parent comp on the child. */ /* Set the parent comp on the child. */
_efl_object_extension_need(comp_pd); _efl_object_extension_need(comp_pd);
@ -1469,6 +1483,8 @@ _efl_object_composite_attach(Eo *parent_id, Efl_Object_Data *pd EINA_UNUSED, Eo
parent->composite_objects = eina_list_prepend(parent->composite_objects, comp_obj_id); parent->composite_objects = eina_list_prepend(parent->composite_objects, comp_obj_id);
EO_OBJ_DONE(parent_id);
EO_OBJ_DONE(comp_obj_id);
return EINA_TRUE; return EINA_TRUE;
} }
@ -1476,10 +1492,19 @@ EOLIAN static Eina_Bool
_efl_object_composite_detach(Eo *parent_id, Efl_Object_Data *pd EINA_UNUSED, Eo *comp_obj_id) _efl_object_composite_detach(Eo *parent_id, Efl_Object_Data *pd EINA_UNUSED, Eo *comp_obj_id)
{ {
EO_OBJ_POINTER_RETURN_VAL(comp_obj_id, comp_obj, EINA_FALSE); EO_OBJ_POINTER_RETURN_VAL(comp_obj_id, comp_obj, EINA_FALSE);
EO_OBJ_POINTER_RETURN_VAL(parent_id, parent, EINA_FALSE); EO_OBJ_POINTER(parent_id, parent);
if (!parent)
{
EO_OBJ_DONE(comp_obj_id);
return EINA_FALSE;
}
if (!efl_composite_part_is(comp_obj_id)) if (!efl_composite_part_is(comp_obj_id))
{
EO_OBJ_DONE(parent_id);
EO_OBJ_DONE(comp_obj_id);
return EINA_FALSE; return EINA_FALSE;
}
parent->composite_objects = eina_list_remove(parent->composite_objects, comp_obj_id); parent->composite_objects = eina_list_remove(parent->composite_objects, comp_obj_id);
/* Clear the comp parent on the child. */ /* Clear the comp parent on the child. */
@ -1490,6 +1515,8 @@ _efl_object_composite_detach(Eo *parent_id, Efl_Object_Data *pd EINA_UNUSED, Eo
_efl_object_extension_noneed(comp_pd); _efl_object_extension_noneed(comp_pd);
} }
EO_OBJ_DONE(parent_id);
EO_OBJ_DONE(comp_obj_id);
return EINA_TRUE; return EINA_TRUE;
} }
@ -1635,6 +1662,7 @@ _efl_object_destructor(Eo *obj, Efl_Object_Data *pd)
{ {
efl_composite_detach(obj, emb_obj_id); efl_composite_detach(obj, emb_obj_id);
} }
EO_OBJ_DONE(obj);
} }
if (pd->ext && pd->ext->composite_parent) if (pd->ext && pd->ext->composite_parent)

View File

@ -208,6 +208,7 @@ Eo *_eo_header_id_get(const Eo_Header *header)
/* Retrieves the pointer to the object from the id */ /* Retrieves the pointer to the object from the id */
_Eo_Object *_eo_obj_pointer_get(const Eo_Id obj_id); _Eo_Object *_eo_obj_pointer_get(const Eo_Id obj_id);
void _eo_obj_pointer_done(const Eo_Id obj_id);
static inline static inline
Efl_Class *_eo_class_id_get(const _Efl_Class *klass) Efl_Class *_eo_class_id_get(const _Efl_Class *klass)

View File

@ -10,6 +10,7 @@ extern Eina_Thread _efl_object_main_thread;
Eina_TLS _eo_table_data; Eina_TLS _eo_table_data;
Eo_Id_Data *_eo_table_data_shared = NULL; Eo_Id_Data *_eo_table_data_shared = NULL;
Eo_Id_Table_Data *_eo_table_data_shared_data = NULL;
////////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////////
@ -119,11 +120,11 @@ _eo_obj_pointer_get(const Eo_Id obj_id)
} }
else else
{ {
eina_spinlock_take(&(tdata->lock)); eina_lock_take(&(_eo_table_data_shared_data->obj_lock));
if (obj_id == tdata->cache.id) if (obj_id == tdata->cache.id)
{ {
ptr = tdata->cache.object; ptr = tdata->cache.object;
goto shared_ok; return ptr;
} }
// get tag bit to check later down below - pipelining // get tag bit to check later down below - pipelining
@ -147,22 +148,22 @@ _eo_obj_pointer_get(const Eo_Id obj_id)
tdata->cache.object = entry->ptr; tdata->cache.object = entry->ptr;
tdata->cache.id = obj_id; tdata->cache.id = obj_id;
ptr = entry->ptr; ptr = entry->ptr;
goto shared_ok; // yes we return keeping the lock locked. thats why
// you must call _eo_obj_pointer_done() wrapped
// by EO_OBJ_DONE() to release
return ptr;
} }
} }
} }
goto err_shared; goto err_shared;
shared_ok:
eina_spinlock_release(&(tdata->lock));
return ptr;
} }
err_shared_null: err_shared_null:
eina_spinlock_release(&(tdata->lock)); eina_lock_release(&(_eo_table_data_shared_data->obj_lock));
err_null: err_null:
DBG("obj_id is NULL. Possibly unintended access?"); DBG("obj_id is NULL. Possibly unintended access?");
return NULL; return NULL;
err_shared: err_shared:
eina_spinlock_release(&(tdata->lock)); eina_lock_release(&(_eo_table_data_shared_data->obj_lock));
err: err:
_eo_obj_pointer_invalid(obj_id, data, domain); _eo_obj_pointer_invalid(obj_id, data, domain);
return NULL; return NULL;
@ -170,3 +171,13 @@ err:
return (_Eo_Object *) obj_id; return (_Eo_Object *) obj_id;
#endif #endif
} }
void
_eo_obj_pointer_done(const Eo_Id obj_id)
{
#ifdef HAVE_EO_ID
Efl_Id_Domain domain = (obj_id >> SHIFT_DOMAIN) & MASK_DOMAIN;
if (EINA_LIKELY(domain != EFL_ID_DOMAIN_SHARED)) return;
eina_lock_release(&(_eo_table_data_shared_data->obj_lock));
#endif
}

View File

@ -53,6 +53,9 @@ void _eo_pointer_error(const char *msg);
} \ } \
} while (0) } while (0)
#define EO_OBJ_DONE(obj_id) \
_eo_obj_pointer_done((Eo_Id)obj_id)
#else #else
#define EO_OBJ_POINTER(obj_id, obj) \ #define EO_OBJ_POINTER(obj_id, obj) \
@ -97,6 +100,8 @@ void _eo_pointer_error(const char *msg);
EO_MAGIC_RETURN((Eo_Header *) klass, EO_CLASS_EINA_MAGIC); \ EO_MAGIC_RETURN((Eo_Header *) klass, EO_CLASS_EINA_MAGIC); \
} while (0) } while (0)
#define EO_OBJ_DONE(obj_id)
#endif #endif
#ifdef EFL_DEBUG #ifdef EFL_DEBUG
@ -108,6 +113,7 @@ extern Eina_TLS _eo_table_data;
#include "eo_ptr_indirection.x" #include "eo_ptr_indirection.x"
extern Eo_Id_Data *_eo_table_data_shared; extern Eo_Id_Data *_eo_table_data_shared;
extern Eo_Id_Table_Data *_eo_table_data_shared_data;
#endif #endif

View File

@ -269,8 +269,6 @@ struct _Eo_Id_Table_Data
cache; cache;
/* Next generation to use when assigning a new entry to a Eo pointer */ /* Next generation to use when assigning a new entry to a Eo pointer */
Generation_Counter generation; Generation_Counter generation;
/* Optional lock around objects and eoid table - only used if shared */
Eina_Spinlock lock;
/* Optional lock around all objects in eoid table - only used if shared */ /* Optional lock around all objects in eoid table - only used if shared */
Eina_Lock obj_lock; Eina_Lock obj_lock;
/* are we shared so we need lock/unlock? */ /* are we shared so we need lock/unlock? */
@ -287,6 +285,7 @@ struct _Eo_Id_Data
extern Eina_TLS _eo_table_data; extern Eina_TLS _eo_table_data;
extern Eo_Id_Data *_eo_table_data_shared; extern Eo_Id_Data *_eo_table_data_shared;
extern Eo_Id_Table_Data *_eo_table_data_shared_data;
static inline Eo_Id_Table_Data * static inline Eo_Id_Table_Data *
_eo_table_data_table_new(Efl_Id_Domain domain) _eo_table_data_table_new(Efl_Id_Domain domain)
@ -297,14 +296,8 @@ _eo_table_data_table_new(Efl_Id_Domain domain)
if (!tdata) return NULL; if (!tdata) return NULL;
if (domain == EFL_ID_DOMAIN_SHARED) if (domain == EFL_ID_DOMAIN_SHARED)
{ {
if (!eina_spinlock_new(&(tdata->lock)))
{
free(tdata);
return NULL;
}
if (!eina_lock_recursive_new(&(tdata->obj_lock))) if (!eina_lock_recursive_new(&(tdata->obj_lock)))
{ {
eina_spinlock_free(&(tdata->lock));
free(tdata); free(tdata);
return NULL; return NULL;
} }
@ -326,19 +319,14 @@ _eo_table_data_new(Efl_Id_Domain domain)
data->tables[data->local_domain] = data->tables[data->local_domain] =
_eo_table_data_table_new(data->local_domain); _eo_table_data_table_new(data->local_domain);
if (domain != EFL_ID_DOMAIN_SHARED) if (domain != EFL_ID_DOMAIN_SHARED)
data->tables[EFL_ID_DOMAIN_SHARED] = data->tables[EFL_ID_DOMAIN_SHARED] = _eo_table_data_shared_data;
_eo_table_data_shared->tables[EFL_ID_DOMAIN_SHARED];
return data; return data;
} }
static void static void
_eo_table_data_table_free(Eo_Id_Table_Data *tdata) _eo_table_data_table_free(Eo_Id_Table_Data *tdata)
{ {
if (tdata->shared) if (tdata->shared) eina_lock_free(&(tdata->obj_lock));
{
eina_spinlock_free(&(tdata->lock));
eina_lock_free(&(tdata->obj_lock));
}
free(tdata); free(tdata);
} }
@ -538,7 +526,7 @@ _eo_id_allocate(const _Eo_Object *obj, const Eo *parent_id)
} }
else else
{ {
eina_spinlock_take(&(tdata->lock)); eina_lock_take(&(_eo_table_data_shared_data->obj_lock));
if (tdata->current_table) if (tdata->current_table)
entry = _get_available_entry(tdata->current_table); entry = _get_available_entry(tdata->current_table);
@ -564,7 +552,7 @@ _eo_id_allocate(const _Eo_Object *obj, const Eo *parent_id)
EFL_ID_DOMAIN_SHARED, EFL_ID_DOMAIN_SHARED,
entry->generation); entry->generation);
shared_err: shared_err:
eina_spinlock_release(&(tdata->lock)); eina_lock_release(&(_eo_table_data_shared_data->obj_lock));
} }
return id; return id;
#else #else
@ -643,7 +631,7 @@ _eo_id_release(const Eo_Id obj_id)
} }
else else
{ {
eina_spinlock_take(&(tdata->lock)); eina_lock_take(&(_eo_table_data_shared_data->obj_lock));
// Check the validity of the entry // Check the validity of the entry
if (tdata->eo_ids_tables[mid_table_id] && (table = TABLE_FROM_IDS)) if (tdata->eo_ids_tables[mid_table_id] && (table = TABLE_FROM_IDS))
{ {
@ -687,11 +675,11 @@ _eo_id_release(const Eo_Id obj_id)
tdata->cache.klass = NULL;; tdata->cache.klass = NULL;;
tdata->cache.isa = EINA_FALSE; tdata->cache.isa = EINA_FALSE;
} }
eina_spinlock_release(&(tdata->lock)); eina_lock_release(&(_eo_table_data_shared_data->obj_lock));
return; return;
} }
} }
eina_spinlock_release(&(tdata->lock)); eina_lock_release(&(_eo_table_data_shared_data->obj_lock));
} }
ERR("obj_id %p is not pointing to a valid object. Maybe it has already been freed.", (void *)obj_id); ERR("obj_id %p is not pointing to a valid object. Maybe it has already been freed.", (void *)obj_id);
#else #else