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

diff --git a/src/lib/eo/eo.c b/src/lib/eo/eo.c
index f147f0d..50b8e66 100644 (file)
--- 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..92e10af 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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);