jpeg pushed a commit to branch master.

http://git.enlightenment.org/core/efl.git/commit/?id=6a4c84a6a48a7d02c52a1556b0ab6ded6c2de081

commit 6a4c84a6a48a7d02c52a1556b0ab6ded6c2de081
Author: Jean-Philippe Andre <[email protected]>
Date:   Mon Jan 13 14:21:44 2014 +0900

    Evas/cserve2: Fix crash in server on file change
    
    An inotify callback is triggered when an image file changes,
    and it is supposed to cleanup all references to this image.
    
    Unfortunately, it was doing it in a very unsafe way as pointers
    could become invalid, because of hash free callbacks in the
    referenced image list. Add some list safety and always assume
    the pointer might be dead after free operations.
    
    TBH, this _file_changed_cb function looks very confused to me
    (as it tries to bypass eina_hash_del).
---
 src/bin/evas/evas_cserve2_cache.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/src/bin/evas/evas_cserve2_cache.c 
b/src/bin/evas/evas_cserve2_cache.c
index bc12fed..f4ce05e 100644
--- a/src/bin/evas/evas_cserve2_cache.c
+++ b/src/bin/evas/evas_cserve2_cache.c
@@ -1673,26 +1673,27 @@ _file_changed_cb(const char *path EINA_UNUSED, 
Eina_Bool deleted EINA_UNUSED, vo
 {
    File_Watch *fw = data;
    File_Entry *fentry;
-   Eina_List *l;
+   Eina_List *l, *l_next;
 
-   EINA_LIST_FOREACH(fw->entries, l, fentry)
+   EINA_LIST_FOREACH_SAFE(fw->entries, l, l_next, fentry)
      {
-        Eina_List *ll;
+        Eina_List *ll, *ll_next;
         Image_Entry *ie;
         File_Data *fd;
+        int fentry_id = ENTRYID(fentry);
 
         fentry->watcher = NULL;
 
-        EINA_LIST_FOREACH(fentry->images, ll, ie)
+        EINA_LIST_FOREACH_SAFE(fentry->images, ll, ll_next, ie)
           {
              Image_Data *idata;
 
              idata = _image_data_find(ENTRYID(ie));
-             eina_hash_set(image_entries, &ENTRYID(ie), NULL);
              if (ASENTRY(ie)->request /*&& !ie->base.request->processing*/)
                cserve2_request_cancel_all(ASENTRY(ie)->request,
                                           CSERVE2_FILE_CHANGED);
              ASENTRY(ie)->request = NULL;
+             eina_hash_set(image_entries, &ENTRYID(ie), NULL);
 
              if (idata)
                {
@@ -1702,8 +1703,10 @@ _file_changed_cb(const char *path EINA_UNUSED, Eina_Bool 
deleted EINA_UNUSED, vo
                }
           }
 
+        // from this point, fentry might have died already
+        // clean up if it's still alive somewhere
 
-        fd = _file_data_find(ENTRYID(fentry));
+        fd = _file_data_find(fentry_id);
         if (fd)
           {
              fd->invalid = EINA_TRUE;
@@ -1711,15 +1714,19 @@ _file_changed_cb(const char *path EINA_UNUSED, 
Eina_Bool deleted EINA_UNUSED, vo
              eina_hash_set(file_entries, &fd->id, NULL);
           }
 
-        if (ASENTRY(fentry)->request
-            /*&& !ASENTRY(fentry)->request->processing*/)
+        fentry = (File_Entry *) eina_hash_find(file_entries, &fentry_id);
+        if (fentry)
           {
-             cserve2_request_cancel_all(ASENTRY(fentry)->request,
-                                        CSERVE2_FILE_CHANGED);
-             ASENTRY(fentry)->request = NULL;
+             if (ASENTRY(fentry)->request
+                 /*&& !ASENTRY(fentry)->request->processing*/)
+               {
+                  cserve2_request_cancel_all(ASENTRY(fentry)->request,
+                                             CSERVE2_FILE_CHANGED);
+                  ASENTRY(fentry)->request = NULL;
+               }
+             if (!fentry->images && !ASENTRY(fentry)->references)
+               _hash_file_entry_free(fentry);
           }
-        if (!fentry->images && !ASENTRY(fentry)->references)
-          _hash_file_entry_free(fentry);
      }
 
    eina_hash_del_by_key(file_watch, fw->path);

-- 


Reply via email to