forked from enlightenment/efl
Eo refcount: Split the refcount to private and public (user).
This commit changes the way refcount is dealt with internally. Before this commit, there was one refcount shared between Eo internals and users. Now there is a refcount for eo operations (like for example, function calls) and one for user refcount (eo_ref). An example bug that this protects against (which is seemingly rather common) is: some_eo_func(obj); // Inside the implementation of that func: pd->a = 1; // The object's private data eo_unref(obj); // To delete the object eo_unref(obj); // A big one extra unref pd->a = 2; // Segfault, this data has already been freed This is a feature, but really just a fix for a class of bugs. @feature
This commit is contained in:
parent
41abda37f6
commit
a5eb66edd4
|
@ -697,19 +697,11 @@ _eo_add_internal_start(const char *file, int line, const Eo_Class *klass_id, Eo
|
||||||
|
|
||||||
_eo_condtor_reset(obj);
|
_eo_condtor_reset(obj);
|
||||||
|
|
||||||
_eo_ref(obj);
|
eo_ref(eo_id);
|
||||||
|
|
||||||
|
/* Reference for the parent if is_ref is done in _eo_add_end */
|
||||||
eo_parent_set(eo_id, parent_id);
|
eo_parent_set(eo_id, parent_id);
|
||||||
|
|
||||||
/* If there's a parent. Ref. Eo_add should return an object with either a
|
|
||||||
* parent ref, or with the lack of, just a ref. */
|
|
||||||
{
|
|
||||||
if (ref && eo_parent_get(eo_id))
|
|
||||||
{
|
|
||||||
_eo_ref(obj);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/* eo_id can change here. Freeing is done on the resolved object. */
|
/* eo_id can change here. Freeing is done on the resolved object. */
|
||||||
eo_id = eo_constructor(eo_id);
|
eo_id = eo_constructor(eo_id);
|
||||||
if (!eo_id)
|
if (!eo_id)
|
||||||
|
@ -724,10 +716,16 @@ _eo_add_internal_start(const char *file, int line, const Eo_Class *klass_id, Eo
|
||||||
}
|
}
|
||||||
else if (eo_id != _eo_obj_id_get(obj))
|
else if (eo_id != _eo_obj_id_get(obj))
|
||||||
{
|
{
|
||||||
|
EO_OBJ_POINTER_RETURN_VAL(eo_id, new_obj, NULL);
|
||||||
/* We have two refs at this point. */
|
/* We have two refs at this point. */
|
||||||
_eo_unref(obj);
|
_eo_unref(obj);
|
||||||
_eo_unref(obj);
|
eo_del((Eo *) obj->header.id);
|
||||||
|
|
||||||
|
_eo_ref(new_obj);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (ref && eo_parent_get(eo_id))
|
||||||
|
{
|
||||||
eo_ref(eo_id);
|
eo_ref(eo_id);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -1374,11 +1372,11 @@ eo_isa(const Eo *eo_id, const Eo_Class *klass_id)
|
||||||
EAPI Eo *
|
EAPI Eo *
|
||||||
eo_xref_internal(const char *file, int line, Eo *obj_id, const Eo *ref_obj_id)
|
eo_xref_internal(const char *file, int line, Eo *obj_id, const Eo *ref_obj_id)
|
||||||
{
|
{
|
||||||
EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, obj_id);
|
eo_ref(obj_id);
|
||||||
|
|
||||||
_eo_ref(obj);
|
|
||||||
|
|
||||||
#ifdef EO_DEBUG
|
#ifdef EO_DEBUG
|
||||||
|
EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, obj_id);
|
||||||
|
|
||||||
Eo_Xref_Node *xref = calloc(1, sizeof(*xref));
|
Eo_Xref_Node *xref = calloc(1, sizeof(*xref));
|
||||||
xref->ref_obj = ref_obj_id;
|
xref->ref_obj = ref_obj_id;
|
||||||
xref->file = file;
|
xref->file = file;
|
||||||
|
@ -1419,7 +1417,7 @@ eo_xunref(Eo *obj_id, const Eo *ref_obj_id)
|
||||||
#else
|
#else
|
||||||
(void) ref_obj_id;
|
(void) ref_obj_id;
|
||||||
#endif
|
#endif
|
||||||
_eo_unref(obj);
|
eo_unref(obj_id);
|
||||||
}
|
}
|
||||||
|
|
||||||
EAPI Eo *
|
EAPI Eo *
|
||||||
|
@ -1427,7 +1425,11 @@ eo_ref(const Eo *obj_id)
|
||||||
{
|
{
|
||||||
EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, (Eo *)obj_id);
|
EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, (Eo *)obj_id);
|
||||||
|
|
||||||
|
++(obj->user_refcount);
|
||||||
|
if (EINA_UNLIKELY(obj->user_refcount == 1))
|
||||||
|
{
|
||||||
_eo_ref(obj);
|
_eo_ref(obj);
|
||||||
|
}
|
||||||
return (Eo *)obj_id;
|
return (Eo *)obj_id;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -1436,15 +1438,25 @@ eo_unref(const Eo *obj_id)
|
||||||
{
|
{
|
||||||
EO_OBJ_POINTER_RETURN(obj_id, obj);
|
EO_OBJ_POINTER_RETURN(obj_id, obj);
|
||||||
|
|
||||||
|
--(obj->user_refcount);
|
||||||
|
if (EINA_UNLIKELY(obj->user_refcount <= 0))
|
||||||
|
{
|
||||||
|
if (obj->user_refcount < 0)
|
||||||
|
{
|
||||||
|
ERR("Obj:%p. User refcount (%d) < 0. Too many unrefs.", obj, obj->user_refcount);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
_eo_unref(obj);
|
_eo_unref(obj);
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
EAPI int
|
EAPI int
|
||||||
eo_ref_get(const Eo *obj_id)
|
eo_ref_get(const Eo *obj_id)
|
||||||
{
|
{
|
||||||
EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, 0);
|
EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, 0);
|
||||||
|
|
||||||
return obj->refcount;
|
return obj->user_refcount;
|
||||||
}
|
}
|
||||||
|
|
||||||
EAPI void
|
EAPI void
|
||||||
|
|
|
@ -552,7 +552,7 @@ _eo_base_parent_set(Eo *obj, Eo_Base_Data *pd, Eo *parent_id)
|
||||||
* the process of deleting the object.*/
|
* the process of deleting the object.*/
|
||||||
if (!parent_id && !eo_obj->del_triggered)
|
if (!parent_id && !eo_obj->del_triggered)
|
||||||
{
|
{
|
||||||
_eo_unref(eo_obj);
|
eo_unref(obj);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -110,6 +110,7 @@ struct _Eo_Object
|
||||||
Eo_Del_Intercept del_intercept;
|
Eo_Del_Intercept del_intercept;
|
||||||
|
|
||||||
short refcount;
|
short refcount;
|
||||||
|
short user_refcount;
|
||||||
#ifdef EO_DEBUG
|
#ifdef EO_DEBUG
|
||||||
short datarefcount;
|
short datarefcount;
|
||||||
#endif
|
#endif
|
||||||
|
@ -322,8 +323,9 @@ _eo_unref(_Eo_Object *obj)
|
||||||
|
|
||||||
if (obj->del_intercept)
|
if (obj->del_intercept)
|
||||||
{
|
{
|
||||||
(obj->refcount)++;
|
Eo *obj_id = _eo_obj_id_get(obj);
|
||||||
obj->del_intercept(_eo_obj_id_get(obj));
|
eo_ref(obj_id);
|
||||||
|
obj->del_intercept(obj_id);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -48,7 +48,7 @@ START_TEST(eo_destructor_unref)
|
||||||
Eo *obj = eo_add(klass, NULL);
|
Eo *obj = eo_add(klass, NULL);
|
||||||
fail_if(!obj);
|
fail_if(!obj);
|
||||||
|
|
||||||
TEST_EO_ERROR("_eo_unref", "Object %p deletion already triggered. You wrongly call eo_unref() within a destructor.");
|
TEST_EO_ERROR("eo_unref", "Obj:%p. User refcount (%d) < 0. Too many unrefs.");
|
||||||
eo_unref(obj);
|
eo_unref(obj);
|
||||||
|
|
||||||
eina_log_print_cb_set(eina_log_print_cb_stderr, NULL);
|
eina_log_print_cb_set(eina_log_print_cb_stderr, NULL);
|
||||||
|
@ -80,7 +80,7 @@ START_TEST(eo_destructor_double_del)
|
||||||
eo_manual_free_set(obj, EINA_TRUE);
|
eo_manual_free_set(obj, EINA_TRUE);
|
||||||
fail_if(!obj);
|
fail_if(!obj);
|
||||||
|
|
||||||
TEST_EO_ERROR("_eo_unref", "Object %p already destructed.");
|
TEST_EO_ERROR("eo_unref", "Obj:%p. User refcount (%d) < 0. Too many unrefs.");
|
||||||
eo_del(obj);
|
eo_del(obj);
|
||||||
eo_del(obj);
|
eo_del(obj);
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue