raster pushed a commit to branch efl-1.22.

http://git.enlightenment.org/core/efl.git/commit/?id=7ea776af2092d57b852f63deb018acd3bd2c5a23

commit 7ea776af2092d57b852f63deb018acd3bd2c5a23
Author: Carsten Haitzler (Rasterman) <ras...@rasterman.com>
Date:   Wed Aug 21 00:32:38 2019 +0100

    eina file refs in edje/evas - audit them and plug holes where refs stay
    
    in 1 situation at least we delete the eina file (close it) but keep
    the ptr around (during destruction) which could cause issues with
    callbaks and events on del and so on.... which may lead to multiple
    closes where only one should happen ... which would explain my invalid
    eina file ref problems i'm seeing. i carefully matched eina file
    handle stores/opens/dups to closes in edje/evas and they seemed to all
    match up so this audit with comments and fixes seems to have plugged
    that now.
    
    @fix
---
 src/lib/edje/edje_edit.c                    |  4 ++--
 src/lib/edje/edje_load.c                    | 10 ++++-----
 src/lib/efl/interfaces/efl_file.c           | 12 ++++++----
 src/lib/elementary/elm_theme.c              | 16 +++++++++-----
 src/lib/evas/cache/evas_cache_image.c       |  6 ++++-
 src/lib/evas/canvas/evas_canvas3d_texture.c |  3 ++-
 src/lib/evas/canvas/evas_image_legacy.c     |  2 +-
 src/lib/evas/canvas/evas_object_image.c     | 34 ++++++++++++++++++++++++-----
 src/lib/evas/common/evas_image_load.c       |  1 +
 src/lib/evas/common/evas_image_main.c       |  6 ++++-
 10 files changed, 68 insertions(+), 26 deletions(-)

diff --git a/src/lib/edje/edje_edit.c b/src/lib/edje/edje_edit.c
index 1c575174ec..969ef843c6 100644
--- a/src/lib/edje/edje_edit.c
+++ b/src/lib/edje/edje_edit.c
@@ -499,14 +499,14 @@ _edje_edit_file_import(Edje *ed, const char *path, const 
char *entry, int compre
    _edje_edit_eet_close(eetf);
 
    eina_file_map_free(f, fdata);
-   eina_file_close(f);
+   eina_file_close(f); // close matching open OK
 
    return EINA_TRUE;
 
 on_error:
    if (eetf) _edje_edit_eet_close(eetf);
    eina_file_map_free(f, fdata);
-   eina_file_close(f);
+   eina_file_close(f); // close matching open OK
 
    return EINA_FALSE;
 }
diff --git a/src/lib/edje/edje_load.c b/src/lib/edje/edje_load.c
index d6cda4277d..8016e6fab2 100644
--- a/src/lib/edje/edje_load.c
+++ b/src/lib/edje/edje_load.c
@@ -272,7 +272,7 @@ edje_file_collection_list(const char *file)
 
    lst = edje_mmap_collection_list(f);
 
-   eina_file_close(f);
+   eina_file_close(f); // close matching open OK
 err:
    free(tmp);
    return lst;
@@ -447,7 +447,7 @@ edje_file_group_exists(const char *file, const char *glob)
 
    result = edje_mmap_group_exists(f, glob);
 
-   eina_file_close(f);
+   eina_file_close(f); // close matching open OK
 err:
    free(tmp);
 
@@ -484,14 +484,14 @@ edje_file_data_get(const char *file, const char *key)
    if (!key) return NULL;
 
    tmp = eina_vpath_resolve(file);
-   f = eina_file_open(file, EINA_FALSE);
+   f = eina_file_open(tmp, EINA_FALSE);
    if (!f)
      {
         ERR("File [%s] can not be opened.", file);
         goto err;
      }
    str = edje_mmap_data_get(f, key);
-   eina_file_close(f);
+   eina_file_close(f); // close matching open OK
 err:
    free(tmp);
 
@@ -2341,7 +2341,7 @@ _edje_file_free(Edje_File *edf)
    if (edf->free_strings) eina_stringshare_del(edf->id);
    _edje_textblock_style_cleanup(edf);
    if (edf->ef) eet_close(edf->ef);
-   if (edf->f) eina_file_close(edf->f);
+   if (edf->f) eina_file_close(edf->f);  // close matching open (in 
_edje_file_open) OK
    free(edf);
 }
 
diff --git a/src/lib/efl/interfaces/efl_file.c 
b/src/lib/efl/interfaces/efl_file.c
index 67619039c9..846c88b9db 100644
--- a/src/lib/efl/interfaces/efl_file.c
+++ b/src/lib/efl/interfaces/efl_file.c
@@ -22,7 +22,8 @@ _efl_file_unload(Eo *obj, Efl_File_Data *pd)
    if (!pd->file) return;
    if (!pd->file_opened) return;
    pd->setting = 1;
-   eina_file_close(pd->file);
+   eina_file_close(pd->file); // close matching open (dup in 
_efl_file_mmap_set) OK
+   pd->file = NULL;
    efl_file_mmap_set(obj, NULL);
    pd->setting = 0;
    pd->loaded = pd->file_opened = EINA_FALSE;
@@ -46,7 +47,7 @@ _efl_file_load(Eo *obj, Efl_File_Data *pd)
         ret = efl_file_mmap_set(obj, f);
         pd->setting = 0;
         if (ret) pd->file_opened = EINA_FALSE;
-        eina_file_close(f);
+        eina_file_close(f); // close matching open OK
      }
    pd->loaded = !ret;
    return ret;
@@ -64,7 +65,7 @@ _efl_file_mmap_set(Eo *obj, Efl_File_Data *pd, const 
Eina_File *f)
         file = eina_file_dup(f);
         if (!file) return errno;
      }
-   if (pd->file) eina_file_close(pd->file);
+   if (pd->file) eina_file_close(pd->file); // close matching open (dup above) 
OK
    pd->file = file;
    pd->loaded = EINA_FALSE;
    
@@ -138,7 +139,10 @@ _efl_file_efl_object_destructor(Eo *obj, Efl_File_Data *pd)
 {
    eina_stringshare_del(pd->vpath);
    eina_stringshare_del(pd->key);
-   eina_file_close(pd->file);
+   eina_file_close(pd->file); // close matching open (dup in 
_efl_file_mmap_set) OK
+   pd->vpath = NULL;
+   pd->key = NULL;
+   pd->file = NULL;
    efl_destructor(efl_super(obj, EFL_FILE_MIXIN));
 }
 
diff --git a/src/lib/elementary/elm_theme.c b/src/lib/elementary/elm_theme.c
index 2a6d52f6fc..13f41e1abc 100644
--- a/src/lib/elementary/elm_theme.c
+++ b/src/lib/elementary/elm_theme.c
@@ -62,7 +62,7 @@ _elm_theme_item_finalize(Eina_Inlist **files,
 
         if (!(version = edje_mmap_data_get(f, "version")))
           {
-             eina_file_close(f);
+             eina_file_close(f); // close matching open (finalize expected to 
consume eina file) OK
              return;
           }
         v = atoi(version);
@@ -75,7 +75,7 @@ _elm_theme_item_finalize(Eina_Inlist **files,
    etf = calloc(1, sizeof(Elm_Theme_File));
    EINA_SAFETY_ON_NULL_RETURN(etf);
    etf->item = eina_stringshare_add(item);
-   etf->handle = f;
+   etf->handle = f; // now own/consume the file handle
    if (istheme)
      {
         name = edje_mmap_data_get(f, "efl_theme_base");
@@ -156,7 +156,7 @@ _elm_theme_file_item_del(Eina_Inlist **files, const char 
*str)
    EINA_INLIST_FOREACH_SAFE(*files, l, item)
      {
         if (item->item != str) continue;
-        eina_file_close(item->handle);
+        eina_file_close(item->handle); // close matching open (file consumed 
in finalize by putting it in etf) OK
         eina_stringshare_del(item->item);
         *files = eina_inlist_remove(*files, EINA_INLIST_GET(item));
         free(item);
@@ -174,7 +174,7 @@ _elm_theme_file_mmap_del(Eina_Inlist **files, const 
Eina_File *file)
    EINA_INLIST_FOREACH_SAFE(*files, l, item)
      {
         if (item->handle != file) continue;
-        eina_file_close(item->handle);
+        eina_file_close(item->handle); // close matching open (file consumed 
in finalize by putting it in etf) OK
         eina_stringshare_del(item->item);
         *files = eina_inlist_remove(*files, EINA_INLIST_GET(item));
         free(item);
@@ -189,7 +189,7 @@ _elm_theme_file_clean(Eina_Inlist **files)
         Elm_Theme_File *etf = EINA_INLIST_CONTAINER_GET(*files, 
Elm_Theme_File);
 
         eina_stringshare_del(etf->item);
-        eina_file_close(etf->handle);
+        eina_file_close(etf->handle); // close matching open (file consumed in 
finalize by putting it in etf) OK
         eina_stringshare_del(etf->match_theme);
         *files = eina_inlist_remove(*files, *files);
         free(etf);
@@ -655,7 +655,11 @@ elm_theme_overlay_mmap_add(Elm_Theme *th, const Eina_File 
*f)
    Eina_File *file = eina_file_dup(f);
 
    if (!th) th = theme_default;
-   if (!th) return;
+   if (!th)
+     {
+        eina_file_close(file); // close matching open (finalize expected to 
consume eina file) OK
+        return;
+     }
    th->overlay_items = eina_list_free(th->overlay_items);
    _elm_theme_item_finalize(&th->overlay, eina_file_filename_get(file), file, 
EINA_TRUE, EINA_FALSE);
    elm_theme_flush(th);
diff --git a/src/lib/evas/cache/evas_cache_image.c 
b/src/lib/evas/cache/evas_cache_image.c
index adc38a86ec..573a8edd20 100644
--- a/src/lib/evas/cache/evas_cache_image.c
+++ b/src/lib/evas/cache/evas_cache_image.c
@@ -185,7 +185,11 @@ _evas_cache_image_entry_delete(Evas_Cache_Image *cache, 
Image_Entry *ie)
    FREESTRC(ie->cache_key);
    FREESTRC(ie->file);
    FREESTRC(ie->key);
-   if (ie->f && ie->flags.given_mmap) eina_file_close(ie->f);
+   if (ie->f && ie->flags.given_mmap)
+     {
+        eina_file_close(ie->f); // close matching open (in 
_evas_cache_image_entry_new) OK
+        ie->f = NULL;
+     }
    ie->cache = NULL;
    if ((cache) && (cache->func.surface_delete)) cache->func.surface_delete(ie);
 
diff --git a/src/lib/evas/canvas/evas_canvas3d_texture.c 
b/src/lib/evas/canvas/evas_canvas3d_texture.c
index 5533e18373..7154e7e7a1 100644
--- a/src/lib/evas/canvas/evas_canvas3d_texture.c
+++ b/src/lib/evas/canvas/evas_canvas3d_texture.c
@@ -334,7 +334,8 @@ _evas_canvas3d_texture_efl_object_constructor(Eo *obj, 
Evas_Canvas3D_Texture_Dat
 EOLIAN static void
 _evas_canvas3d_texture_efl_object_destructor(Eo *obj, 
Evas_Canvas3D_Texture_Data *pd)
 {
-   eina_file_close(pd->f);
+   eina_file_close(pd->f); // close matching open (matches open in 
_evas_canvas3d_texture_efl_file_load) OK
+   pd->f = NULL;
    _texture_fini(obj);
    efl_destructor(efl_super(obj, MY_CLASS));
 }
diff --git a/src/lib/evas/canvas/evas_image_legacy.c 
b/src/lib/evas/canvas/evas_image_legacy.c
index 6d466be575..441524e424 100644
--- a/src/lib/evas/canvas/evas_image_legacy.c
+++ b/src/lib/evas/canvas/evas_image_legacy.c
@@ -50,7 +50,7 @@ evas_object_image_memfile_set(Evas_Object *eo_obj, void 
*data, int size, char *f
    f = eina_file_virtualize(NULL, data, size, EINA_TRUE);
    if (!f) return ;
    efl_file_simple_mmap_load(eo_obj, f, key);
-   eina_file_close(f);
+   eina_file_close(f); // close matching open OK
 }
 
 EAPI void
diff --git a/src/lib/evas/canvas/evas_object_image.c 
b/src/lib/evas/canvas/evas_object_image.c
index dd8041b35a..780dcfe8d6 100644
--- a/src/lib/evas/canvas/evas_object_image.c
+++ b/src/lib/evas/canvas/evas_object_image.c
@@ -257,7 +257,7 @@ _evas_image_init_set(const Eina_File *f, const char *key,
       state_write->f = NULL;
 
       if (f) state_write->f = eina_file_dup(f);
-      eina_file_close(tmp);
+      eina_file_close(tmp); // close matching open (dup above) OK
 
       eina_stringshare_replace(&state_write->key, key);
       state_write->opaque_valid = 0;
@@ -1354,10 +1354,34 @@ evas_object_image_free(Evas_Object *eo_obj, 
Evas_Object_Protected_Data *obj)
    Eina_Rectangle *r;
 
    /* free obj */
-   eina_file_close(o->cur->f);
-   if (o->cur->key) eina_stringshare_del(o->cur->key);
-   if (o->cur->source) _evas_image_proxy_unset(eo_obj, obj, o);
-   if (o->cur->scene) _evas_image_3d_unset(eo_obj, obj, o);
+   if (o->cur->key)
+     {
+        eina_stringshare_del(o->cur->key);
+        EINA_COW_IMAGE_STATE_WRITE_BEGIN(o, state_write)
+        state_write->key = NULL;
+        EINA_COW_IMAGE_STATE_WRITE_END(o, state_write);
+     }
+   if (o->cur->source)
+     {
+        if (o->cur->source) _evas_image_proxy_unset(eo_obj, obj, o);
+        EINA_COW_IMAGE_STATE_WRITE_BEGIN(o, state_write)
+        state_write->source = NULL;
+        EINA_COW_IMAGE_STATE_WRITE_END(o, state_write);
+     }
+   if (o->cur->scene)
+     {
+        if (o->cur->scene) _evas_image_3d_unset(eo_obj, obj, o);
+        EINA_COW_IMAGE_STATE_WRITE_BEGIN(o, state_write)
+        state_write->scene = NULL;
+        EINA_COW_IMAGE_STATE_WRITE_END(o, state_write);
+     }
+   if (o->cur->f)
+     {
+        eina_file_close(o->cur->f); // close matching open (dup in 
_evas_image_init_set) OK
+        EINA_COW_IMAGE_STATE_WRITE_BEGIN(o, state_write)
+        state_write->f = NULL;
+        EINA_COW_IMAGE_STATE_WRITE_END(o, state_write);
+     }
    if (obj->layer && obj->layer->evas)
      {
         if (o->engine_data && ENC)
diff --git a/src/lib/evas/common/evas_image_load.c 
b/src/lib/evas/common/evas_image_load.c
index 7d4ca66790..5d7afafe43 100644
--- a/src/lib/evas/common/evas_image_load.c
+++ b/src/lib/evas/common/evas_image_load.c
@@ -191,6 +191,7 @@ _evas_image_file_header(Evas_Module *em, Image_Entry *ie, 
int *error)
         if (!ie->f)
           {
              ie->f = eina_file_open(ie->file, EINA_FALSE);
+             ie->flags.given_mmap = EINA_FALSE;
              file = ie->file;
           }
         else file = eina_file_filename_get(ie->f);
diff --git a/src/lib/evas/common/evas_image_main.c 
b/src/lib/evas/common/evas_image_main.c
index a9cee630aa..30cd6e9ace 100644
--- a/src/lib/evas/common/evas_image_main.c
+++ b/src/lib/evas/common/evas_image_main.c
@@ -518,7 +518,11 @@ _evas_common_rgba_image_delete(Image_Entry *ie)
              free(frame);
           }
      }
-   if (ie->f && !ie->flags.given_mmap) eina_file_close(ie->f);
+   if (ie->f && !ie->flags.given_mmap)
+     {
+        eina_file_close(ie->f); // close matching open (dup in 
_evas_image_file_header) OK
+        ie->f = NULL;
+     }
    eina_freeq_ptr_add(eina_freeq_main_get(), im, free, sizeof(*im));
 }
 

-- 


Reply via email to