On Wed, 07 Sep 2016 11:43:12 -0700 Cedric BAIL <[email protected]> said:
actually on 2nd thought.. this is all hiding a nice big fat race condition. FAT. _efl_ui_image_async_file_set() will spawn a thread. ecore_thread_run(). now.... let's pretend IMMEDIATELY we del the object: efl_ui_image_async_file_set(obj, "bla.png", NULL, NULL); efl_del(obj); thread is still spinning up getting started and now it accesses a sd pointer that now has already been freed! the above design has a major problem here. you need to refcount the object up one until thread result is back to keep it alive. this now creates some new challenges like how do you delete a window with such an obj when some thread is taking 3 seconds to go open some image... ? the code here is "janky" imho. the best way is to store the async file/key in a small struct that is malloced, include the Eo *obj in there too but dont ACCESS it from the thread, then when it's done SEND the same ptr back as thread result and free in done/fail etc. handlers in the mainloop and then call cb's from there. set the Eo *obj as a wref so when it comes back if obj is NULL, free it but don't call any cb. so this code is still "janky" and has bugs waiting to happen. glad the new eo stuff brought it out :) > cedric pushed a commit to branch master. > > http://git.enlightenment.org/core/efl.git/commit/?id=e6a810efa75ac4e6941964a8737beb03f83bb2fa > > commit e6a810efa75ac4e6941964a8737beb03f83bb2fa > Author: Cedric BAIL <[email protected]> > Date: Wed Sep 7 11:42:29 2016 -0700 > > elementary: fix call to efl_data_scope_get from another thread. > > This should fix the breakage on elm_suite. > --- > src/lib/elementary/efl_ui_image.c | 30 +++++++++++++----------------- > src/lib/elementary/efl_ui_widget_image.h | 1 + > 2 files changed, 14 insertions(+), 17 deletions(-) > > diff --git a/src/lib/elementary/efl_ui_image.c > b/src/lib/elementary/efl_ui_image.c index 3c697aa..0ddab23 100644 > --- a/src/lib/elementary/efl_ui_image.c > +++ b/src/lib/elementary/efl_ui_image.c > @@ -243,14 +243,12 @@ _async_open_data_free(Async_Open_Data *data) > static void > _efl_ui_image_async_open_do(void *data, Ecore_Thread *thread EINA_UNUSED) > { > - Evas_Object *obj = data; > + Efl_Ui_Image_Data * sd = data; > Eina_File *f; > Async_Open_Data *todo, *done; > void *map = NULL; > unsigned int buf; > > - EFL_UI_IMAGE_DATA_GET(obj, sd); > - > eina_spinlock_take(&sd->async.lck); > todo = sd->async.todo; > done = sd->async.done; > @@ -332,8 +330,7 @@ begin: > static void > _efl_ui_image_async_open_cancel(void *data, Ecore_Thread *thread EINA_UNUSED) > { > - Evas_Object *obj = data; > - EFL_UI_IMAGE_DATA_GET(obj, sd); > + Efl_Ui_Image_Data * sd = data; > > DBG("Async open thread was canceled"); > sd->async.th = NULL; > @@ -345,15 +342,13 @@ _efl_ui_image_async_open_cancel(void *data, > Ecore_Thread *thread EINA_UNUSED) static void > _efl_ui_image_async_open_done(void *data, Ecore_Thread *thread EINA_UNUSED) > { > - Evas_Object *obj = data; > + Efl_Ui_Image_Data * sd = data; > Eina_Stringshare *file, *key; > Async_Open_Data *done; > Eina_Bool ok; > Eina_File *f; > void *map; > > - EFL_UI_IMAGE_DATA_GET(obj, sd); > - > // async open thread can't be running now > // locking anyways to be sure (memory barrier), see CID 1343345 > eina_spinlock_take(&sd->async.lck); > @@ -366,7 +361,7 @@ _efl_ui_image_async_open_done(void *data, Ecore_Thread > *thread EINA_UNUSED) eina_spinlock_release(&sd->async.lck); > sd->async.th = ecore_thread_run(_efl_ui_image_async_open_do, > _efl_ui_image_async_open_done, > - _efl_ui_image_async_open_cancel, > obj); > + _efl_ui_image_async_open_cancel, sd); > > return; > } > @@ -380,7 +375,7 @@ _efl_ui_image_async_open_done(void *data, Ecore_Thread > *thread EINA_UNUSED) ERR("Async open failed for an unknown reason."); > sd->async_failed = EINA_TRUE; > eina_spinlock_release(&sd->async.lck); > - efl_event_callback_legacy_call(obj, EFL_FILE_EVENT_ASYNC_ERROR, > NULL); > + efl_event_callback_legacy_call(sd->self, EFL_FILE_EVENT_ASYNC_ERROR, > NULL); return; > } > > @@ -401,7 +396,7 @@ _efl_ui_image_async_open_done(void *data, Ecore_Thread > *thread EINA_UNUSED) if (sd->edje) > ok = edje_object_mmap_set(sd->img, f, key); > else > - ok = _efl_ui_image_smart_internal_file_set(obj, sd, file, f, key); > + ok = _efl_ui_image_smart_internal_file_set(sd->self, sd, file, f, > key); } > > if (ok) > @@ -410,15 +405,15 @@ _efl_ui_image_async_open_done(void *data, Ecore_Thread > *thread EINA_UNUSED) sd->async_failed = EINA_FALSE; > ELM_SAFE_FREE(sd->async.file, eina_stringshare_del); > ELM_SAFE_FREE(sd->async.key, eina_stringshare_del); > - efl_event_callback_legacy_call(obj, EFL_FILE_EVENT_ASYNC_OPENED, > NULL); > - _efl_ui_image_internal_sizing_eval(obj, sd); > + efl_event_callback_legacy_call(sd->self, > EFL_FILE_EVENT_ASYNC_OPENED, NULL); > + _efl_ui_image_internal_sizing_eval(sd->self, sd); > } > else > { > // TODO: Implement Efl.File async,error event_info type > sd->async_failed = EINA_TRUE; > // keep file,key around for file_get > - efl_event_callback_legacy_call(obj, EFL_FILE_EVENT_ASYNC_ERROR, > NULL); > + efl_event_callback_legacy_call(sd->self, EFL_FILE_EVENT_ASYNC_ERROR, > NULL); } > > // close f, map and free strings > @@ -426,8 +421,8 @@ _efl_ui_image_async_open_done(void *data, Ecore_Thread > *thread EINA_UNUSED) } > > static Eina_Bool > -_efl_ui_image_async_file_set(Eo *obj, Efl_Ui_Image_Data *sd, > - const char *file, const Eina_File *f, const char > *key) +_efl_ui_image_async_file_set(Eo *obj EINA_UNUSED, Efl_Ui_Image_Data > *sd, > + const char *file, const Eina_File *f, const > char *key) { > Async_Open_Data *todo; > Eina_Bool was_running; > @@ -448,7 +443,7 @@ _efl_ui_image_async_file_set(Eo *obj, Efl_Ui_Image_Data > *sd, was_running = EINA_FALSE; > sd->async.th = ecore_thread_run(_efl_ui_image_async_open_do, > _efl_ui_image_async_open_done, > - _efl_ui_image_async_open_cancel, > obj); > + _efl_ui_image_async_open_cancel, sd); > } > else was_running = EINA_TRUE; > > @@ -882,6 +877,7 @@ _efl_ui_image_efl_object_constructor(Eo *obj, > Efl_Ui_Image_Data *pd) elm_interface_atspi_accessible_role_set(obj, > ELM_ATSPI_ROLE_IMAGE); > pd->scale_type = EFL_UI_IMAGE_SCALE_TYPE_FIT_INSIDE; > + pd->self = obj; > > return obj; > } > diff --git a/src/lib/elementary/efl_ui_widget_image.h > b/src/lib/elementary/efl_ui_widget_image.h index a0da68a..209d65a 100644 > --- a/src/lib/elementary/efl_ui_widget_image.h > +++ b/src/lib/elementary/efl_ui_widget_image.h > @@ -43,6 +43,7 @@ typedef enum > typedef struct _Efl_Ui_Image_Data Efl_Ui_Image_Data; > struct _Efl_Ui_Image_Data > { > + Eo *self; > Evas_Object *hit_rect; > Evas_Object *img; > Evas_Object *prev_img; > > -- > > -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- The Rasterman (Carsten Haitzler) [email protected] ------------------------------------------------------------------------------ _______________________________________________ enlightenment-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
