tasn pushed a commit to branch master.
http://git.enlightenment.org/core/efl.git/commit/?id=a5eb66edd4245a5198ca0881e3583e8cc415afa3
commit a5eb66edd4245a5198ca0881e3583e8cc415afa3
Author: Tom Hacohen <t...@stosb.com>
Date: Tue Jul 12 10:26:23 2016 +0100
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
---
src/lib/eo/eo.c | 48 ++++++++++++++--------
src/lib/eo/eo_base_class.c | 2 +-
src/lib/eo/eo_private.h | 6 ++-
.../eo/suite/eo_test_class_behaviour_errors.c | 4 +-
4 files changed, 37 insertions(+), 23 deletions(-)
diff --git a/src/lib/eo/eo.c b/src/lib/eo/eo.c
index f147f0d..50b8e66 100644
--- a/src/lib/eo/eo.c
+++ b/src/lib/eo/eo.c
@@ -697,19 +697,11 @@ _eo_add_internal_start(const char *file, int line, const
Eo_Class *klass_id, Eo
_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);
- /* 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 = eo_constructor(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))
{
+ EO_OBJ_POINTER_RETURN_VAL(eo_id, new_obj, NULL);
/* We have two refs at this point. */
_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);
}
@@ -1374,11 +1372,11 @@ eo_isa(const Eo *eo_id, const Eo_Class *klass_id)
EAPI Eo *
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);
+ eo_ref(obj_id);
#ifdef EO_DEBUG
+ EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, obj_id);
+
Eo_Xref_Node *xref = calloc(1, sizeof(*xref));
xref->ref_obj = ref_obj_id;
xref->file = file;
@@ -1419,7 +1417,7 @@ eo_xunref(Eo *obj_id, const Eo *ref_obj_id)
#else
(void) ref_obj_id;
#endif
- _eo_unref(obj);
+ eo_unref(obj_id);
}
EAPI Eo *
@@ -1427,7 +1425,11 @@ eo_ref(const Eo *obj_id)
{
EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, (Eo *)obj_id);
- _eo_ref(obj);
+ ++(obj->user_refcount);
+ if (EINA_UNLIKELY(obj->user_refcount == 1))
+ {
+ _eo_ref(obj);
+ }
return (Eo *)obj_id;
}
@@ -1436,7 +1438,17 @@ eo_unref(const Eo *obj_id)
{
EO_OBJ_POINTER_RETURN(obj_id, obj);
- _eo_unref(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);
+ }
}
EAPI int
@@ -1444,7 +1456,7 @@ eo_ref_get(const Eo *obj_id)
{
EO_OBJ_POINTER_RETURN_VAL(obj_id, obj, 0);
- return obj->refcount;
+ return obj->user_refcount;
}
EAPI void
diff --git a/src/lib/eo/eo_base_class.c b/src/lib/eo/eo_base_class.c
index 2e2fe3f..92e10af6 100644
--- a/src/lib/eo/eo_base_class.c
+++ b/src/lib/eo/eo_base_class.c
@@ -552,7 +552,7 @@ _eo_base_parent_set(Eo *obj, Eo_Base_Data *pd, Eo
*parent_id)
* the process of deleting the object.*/
if (!parent_id && !eo_obj->del_triggered)
{
- _eo_unref(eo_obj);
+ eo_unref(obj);
}
}
diff --git a/src/lib/eo/eo_private.h b/src/lib/eo/eo_private.h
index 9958c82..d47fcca 100644
--- a/src/lib/eo/eo_private.h
+++ b/src/lib/eo/eo_private.h
@@ -110,6 +110,7 @@ struct _Eo_Object
Eo_Del_Intercept del_intercept;
short refcount;
+ short user_refcount;
#ifdef EO_DEBUG
short datarefcount;
#endif
@@ -322,8 +323,9 @@ _eo_unref(_Eo_Object *obj)
if (obj->del_intercept)
{
- (obj->refcount)++;
- obj->del_intercept(_eo_obj_id_get(obj));
+ Eo *obj_id = _eo_obj_id_get(obj);
+ eo_ref(obj_id);
+ obj->del_intercept(obj_id);
return;
}
diff --git a/src/tests/eo/suite/eo_test_class_behaviour_errors.c
b/src/tests/eo/suite/eo_test_class_behaviour_errors.c
index 12c478f..3b25b38 100644
--- a/src/tests/eo/suite/eo_test_class_behaviour_errors.c
+++ b/src/tests/eo/suite/eo_test_class_behaviour_errors.c
@@ -48,7 +48,7 @@ START_TEST(eo_destructor_unref)
Eo *obj = eo_add(klass, NULL);
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);
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);
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);
--
