[E-devel] Evas keydown segfault
Hi all, Mekius recently discovered a nasty segfault when trying to use the Enter key to dismiss a dialog. As it turns out, this affects all dialogs. After some digging into evas_callbacks.c I have come up with the following patch. I didn't commit because I'm not entirely sure that it's the proper solution, tho it does fix the segfault. Here is the backtrace: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread -1215646016 (LWP 5953)] evas_object_event_callback_call (obj=0x857be20, type=EVAS_CALLBACK_KEY_DOWN, event_info=0xbfacbe58) at evas_callbacks.c:259 259 obj->callbacks->walking_list--; #0 evas_object_event_callback_call (obj=0x857be20, type=EVAS_CALLBACK_KEY_DOWN, event_info=0xbfacbe58) at evas_callbacks.c:259 #1 0xb7e09acd in evas_event_feed_key_down (e=0x856a760, keyname=0x85c4db8 "Return", key=0x8215108 "Return", string=0x81d49e8 "\r", compose=0x0, timestamp=55205686, data=0x0) at evas_events.c:820 #2 0xb7ef0480 in _ecore_evas_x_event_key_down (data=0x0, type=10, event=0x85c74a0) at ecore_evas_x.c:472 #3 0xb7ce0bb6 in _ecore_event_call () at ecore_events.c:428 #4 0xb7ce770e in _ecore_main_loop_iterate_internal (once_only=0) at ecore_main.c:639 #5 0xb7ce78f7 in ecore_main_loop_begin () at ecore_main.c:79 #6 0x080683b1 in main (argc=1, argv=0xbfacf954) at e_main.c:875 Continuing. Patch attached. devilhorns Index: evas_callbacks.c === RCS file: /cvs/e/e17/libs/evas/src/lib/canvas/evas_callbacks.c,v retrieving revision 1.30 diff -u -r1.30 evas_callbacks.c --- evas_callbacks.c 5 Apr 2007 15:40:51 - 1.30 +++ evas_callbacks.c 8 Apr 2007 19:45:42 - @@ -243,7 +243,9 @@ default: break; } -obj->callbacks->walking_list++; + if (obj->callbacks) + obj->callbacks->walking_list++; + for (l = *l_mod; l; l = l->next) { Evas_Func_Node *fn; @@ -256,9 +258,12 @@ } if (obj->delete_me) break; } -obj->callbacks->walking_list--; -if (!obj->callbacks->walking_list) - evas_object_event_callback_clear(obj); + if (obj->callbacks) + { + obj->callbacks->walking_list--; + if (!obj->callbacks->walking_list) + evas_object_event_callback_clear(obj); + } if (type == EVAS_CALLBACK_MOUSE_DOWN) { - Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] Evas keydown segfault
On Sun, Apr 08, 2007 at 03:49:08PM -0400, Christopher Michael wrote: > Hi all, > > Mekius recently discovered a nasty segfault when trying to use the Enter > key to dismiss a dialog. As it turns out, this affects all dialogs. > After some digging into evas_callbacks.c I have come up with the > following patch. I didn't commit because I'm not entirely sure that it's > the proper solution, tho it does fix the segfault. > I'm not sure this is a proper fix. It looks like it would just hide a bug that is somewhere else. It seems that one of the callbacks is triggering something that is setting obj->callbacks to NULL. I think that either the place that this happens needs to be modified to honor the 'walking_list' flag on the object, or the list walking needs to be modified to break if obj->callbacks is NULL after calling a cb func. I only had a few minutes to look at this though, and I'm not very familiar with the code. Brian - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] Evas keydown segfault
On Wed, Apr 18, 2007 at 11:54:28PM -0500, Brian Mattern wrote: > On Sun, Apr 08, 2007 at 03:49:08PM -0400, Christopher Michael wrote: > > Hi all, > > > > Mekius recently discovered a nasty segfault when trying to use the Enter > > key to dismiss a dialog. As it turns out, this affects all dialogs. > > After some digging into evas_callbacks.c I have come up with the > > following patch. I didn't commit because I'm not entirely sure that it's > > the proper solution, tho it does fix the segfault. > > > > I'm not sure this is a proper fix. It looks like it would just hide a > bug that is somewhere else. It seems that one of the callbacks is > triggering something that is setting obj->callbacks to NULL. I think > that either the place that this happens needs to be modified to honor > the 'walking_list' flag on the object, or the list walking needs to be > modified to break if obj->callbacks is NULL after calling a cb func. > > I only had a few minutes to look at this though, and I'm not very > familiar with the code. > > Brian > After some more digging, here's what I've come up with: * When you hit enter, the button widget is 'activated' * e_widget_activate() calls the No buttons callback, which deletes the dialog * this frees the evas, which frees its layers, which frees its objects, which happens to set obj->callbacks to NULL * e is still looping through its callbacks at this point, and segfaults * the NULL check in the aforementioned patch happens to work, but only by luck (the object pointer its accessing is now invalid). So, should we require that evas callbacks not free their evas? (E.g. force them to schedule a free and then actually do it outside of any callbacks) Or, should we alter evas to defer evas frees while walking an object's callbacks? Brian - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] Evas keydown segfault
Brian wrote: > After some more digging, here's what I've come up with: > > * When you hit enter, the button widget is 'activated' > * e_widget_activate() calls the No buttons callback, which >deletes the dialog > * this frees the evas, which frees its layers, which frees >its objects, which happens to set obj->callbacks to NULL > * e is still looping through its callbacks at this point, >and segfaults > * the NULL check in the aforementioned patch happens to work, >but only by luck (the object pointer its accessing is now >invalid). > > So, should we require that evas callbacks not free their evas? > (E.g. force them to schedule a free and then actually do it > outside of any callbacks) > > Or, should we alter evas to defer evas frees while walking > an object's callbacks? Ummm... this is really more raster's domain of intimacy :) but I'd say that an evas obj's callback shouldn't free an evas (wether the one it's in or any other), but the latter possibility sounds like it could be reasonable as well.. I'd have to look at this in more detail to give any real suggestion on my part though. jose. - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] Evas keydown segfault
[EMAIL PROTECTED] wrote: Brian wrote: After some more digging, here's what I've come up with: * When you hit enter, the button widget is 'activated' * e_widget_activate() calls the No buttons callback, which deletes the dialog * this frees the evas, which frees its layers, which frees its objects, which happens to set obj->callbacks to NULL * e is still looping through its callbacks at this point, and segfaults * the NULL check in the aforementioned patch happens to work, but only by luck (the object pointer its accessing is now invalid). So, should we require that evas callbacks not free their evas? (E.g. force them to schedule a free and then actually do it outside of any callbacks) Or, should we alter evas to defer evas frees while walking an object's callbacks? Ummm... this is really more raster's domain of intimacy :) but I'd say that an evas obj's callback shouldn't free an evas (wether the one it's in or any other), but the latter possibility sounds like it could be reasonable as well.. I'd have to look at this in more detail to give any real suggestion on my part though. It will be damn hard to prevent an evas event to kill itself. Anyway, this patch works. Please review. Sebastian Index: src/lib/canvas/evas_callbacks.c === RCS file: /cvs/e/e17/libs/evas/src/lib/canvas/evas_callbacks.c,v retrieving revision 1.30 diff -u -r1.30 evas_callbacks.c --- src/lib/canvas/evas_callbacks.c 5 Apr 2007 15:40:51 - 1.30 +++ src/lib/canvas/evas_callbacks.c 19 Apr 2007 17:14:32 - @@ -276,6 +276,7 @@ if ((obj->smart.parent) && (type != EVAS_CALLBACK_FREE) && (type <= EVAS_CALLBACK_KEY_UP)) evas_object_event_callback_call(obj->smart.parent, type, event_info); + if (e->delete_me) evas_free(e); } /** Index: src/lib/canvas/evas_main.c === RCS file: /cvs/e/e17/libs/evas/src/lib/canvas/evas_main.c,v retrieving revision 1.32 diff -u -r1.32 evas_main.c --- src/lib/canvas/evas_main.c 16 Nov 2006 03:20:24 - 1.32 +++ src/lib/canvas/evas_main.c 19 Apr 2007 17:14:32 - @@ -101,6 +101,12 @@ Evas_Object *o; o = (Evas_Object *)ll; + if ((o->callbacks) && (o->callbacks->walking_list)) + { + /* Defer free */ + e->delete_me = 1; + return; + } if (!o->delete_me) del = 1; } Index: src/lib/include/evas_private.h === RCS file: /cvs/e/e17/libs/evas/src/lib/include/evas_private.h,v retrieving revision 1.81 diff -u -r1.81 evas_private.h --- src/lib/include/evas_private.h 21 Feb 2007 21:43:45 - 1.81 +++ src/lib/include/evas_private.h 19 Apr 2007 17:14:33 - @@ -340,6 +340,8 @@ intlast_mouse_down_counter; intlast_mouse_up_counter; Evas_Font_Hinting_Flags hinting; + + unsigned char delete_me : 1; }; struct _Evas_Layer @@ -455,7 +457,7 @@ void (*func) (void *data, Evas *e, Evas_Object *obj, void *event_info); void *data; Evas_Callback_Type type; - char delete_me : 1; + unsigned char delete_me : 1; }; struct _Evas_Data_Node - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] Evas keydown segfault
Sebastian wrote: > > > So, should we require that evas callbacks not free their evas? > > > (E.g. force them to schedule a free and then actually do it > > > outside of any callbacks) > > > > > > Or, should we alter evas to defer evas frees while walking > > > an object's callbacks? > > > >Ummm... this is really more raster's domain of intimacy :) > > but I'd say that an evas obj's callback shouldn't free an evas > > (wether the one it's in or any other), but the latter possibility > > sounds like it could be reasonable as well.. I'd have to look at > > this in more detail to give any real suggestion on my part though. > > It will be damn hard to prevent an evas event to kill itself. > Not prevent an evas event to kill itself, just not have an obj directly execute the evas_free func in one of its callbacks while doing a callback walk.. though I'm not sure that's what was ocurring here at all. Like in either one of Brian's suggestions, it would be best to set such free requests til after all pending callbacks were done. Alternatively, all further callbacks could be interrupted, and then the freeing done. Event handling is really largely dependent on the particular semantics one wants for the timing, propagation, delegation, and execution of requests, so it's not clear to me exactly what should be the 'right thing' here.. wether it "works" or not. jose. - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] Evas keydown segfault
On Thu, 19 Apr 2007 11:25:50 -0500 Brian Mattern <[EMAIL PROTECTED]> babbled: > On Wed, Apr 18, 2007 at 11:54:28PM -0500, Brian Mattern wrote: > > On Sun, Apr 08, 2007 at 03:49:08PM -0400, Christopher Michael wrote: > > > Hi all, > > > > > > Mekius recently discovered a nasty segfault when trying to use the Enter > > > key to dismiss a dialog. As it turns out, this affects all dialogs. > > > After some digging into evas_callbacks.c I have come up with the > > > following patch. I didn't commit because I'm not entirely sure that it's > > > the proper solution, tho it does fix the segfault. > > > > > > > I'm not sure this is a proper fix. It looks like it would just hide a > > bug that is somewhere else. It seems that one of the callbacks is > > triggering something that is setting obj->callbacks to NULL. I think > > that either the place that this happens needs to be modified to honor > > the 'walking_list' flag on the object, or the list walking needs to be > > modified to break if obj->callbacks is NULL after calling a cb func. > > > > I only had a few minutes to look at this though, and I'm not very > > familiar with the code. > > > > Brian > > > > After some more digging, here's what I've come up with: > > * When you hit enter, the button widget is 'activated' > * e_widget_activate() calls the No buttons callback, which deletes the > dialog > * this frees the evas, which frees its layers, which frees its objects, > which happens to set obj->callbacks to NULL > * e is still looping through its callbacks at this point, and segfaults > * the NULL check in the aforementioned patch happens to work, but only by > luck (the object pointer its accessing is now invalid). > > So, should we require that evas callbacks not free their evas? (E.g. > force them to schedule a free and then actually do it outside of any > callbacks) > > Or, should we alter evas to defer evas frees while walking an object's > callbacks? i would go for this option. evas should incriemnt a ref or busy count and then when its finished walking decrement and if the delete_me flag is set - do the actual delete. abort starting any list walks if delete_me is set... > Brian > > > - > This SF.net email is sponsored by DB2 Express > Download DB2 Express C - the FREE version of DB2 express and take > control of your XML. No limits. Just data. Click to get it now. > http://sourceforge.net/powerbar/db2/ > ___ > enlightenment-devel mailing list > enlightenment-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > -- - Codito, ergo sum - "I code, therefore I am" -- The Rasterman (Carsten Haitzler)[EMAIL PROTECTED] 裸好多 Tokyo, Japan (東京 日本) - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] Evas keydown segfault
On Thu, 19 Apr 2007 19:16:36 +0200 Sebastian Dransfeld <[EMAIL PROTECTED]> babbled: almost - patch wise this is close. you likely need a counter so recursive callbacks do the right thing. :) (i.e only do the deferred free once the counter hits 0, and increment counter when you start walking a list, decrement after finishing the walk) > [EMAIL PROTECTED] wrote: > > Brian wrote: > > > >> After some more digging, here's what I've come up with: > >> > >> * When you hit enter, the button widget is 'activated' > >> * e_widget_activate() calls the No buttons callback, which > >>deletes the dialog > >> * this frees the evas, which frees its layers, which frees > >>its objects, which happens to set obj->callbacks to NULL > >> * e is still looping through its callbacks at this point, > >>and segfaults > >> * the NULL check in the aforementioned patch happens to work, > >>but only by luck (the object pointer its accessing is now > >>invalid). > >> > >> So, should we require that evas callbacks not free their evas? > >> (E.g. force them to schedule a free and then actually do it > >> outside of any callbacks) > >> > >> Or, should we alter evas to defer evas frees while walking > >> an object's callbacks? > > > > Ummm... this is really more raster's domain of intimacy :) > > but I'd say that an evas obj's callback shouldn't free an evas > > (wether the one it's in or any other), but the latter possibility > > sounds like it could be reasonable as well.. I'd have to look at > > this in more detail to give any real suggestion on my part though. > > It will be damn hard to prevent an evas event to kill itself. > > Anyway, this patch works. Please review. > > Sebastian > -- - Codito, ergo sum - "I code, therefore I am" -- The Rasterman (Carsten Haitzler)[EMAIL PROTECTED] 裸好多 Tokyo, Japan (東京 日本) - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] Evas keydown segfault
On Thu, 19 Apr 2007 19:16:36 +0200 Sebastian Dransfeld <[EMAIL PROTECTED]> babbled: got a more extensive fix in cvs now. :) > [EMAIL PROTECTED] wrote: > > Brian wrote: > > > >> After some more digging, here's what I've come up with: > >> > >> * When you hit enter, the button widget is 'activated' > >> * e_widget_activate() calls the No buttons callback, which > >>deletes the dialog > >> * this frees the evas, which frees its layers, which frees > >>its objects, which happens to set obj->callbacks to NULL > >> * e is still looping through its callbacks at this point, > >>and segfaults > >> * the NULL check in the aforementioned patch happens to work, > >>but only by luck (the object pointer its accessing is now > >>invalid). > >> > >> So, should we require that evas callbacks not free their evas? > >> (E.g. force them to schedule a free and then actually do it > >> outside of any callbacks) > >> > >> Or, should we alter evas to defer evas frees while walking > >> an object's callbacks? > > > > Ummm... this is really more raster's domain of intimacy :) > > but I'd say that an evas obj's callback shouldn't free an evas > > (wether the one it's in or any other), but the latter possibility > > sounds like it could be reasonable as well.. I'd have to look at > > this in more detail to give any real suggestion on my part though. > > It will be damn hard to prevent an evas event to kill itself. > > Anyway, this patch works. Please review. > > Sebastian > -- - Codito, ergo sum - "I code, therefore I am" -- The Rasterman (Carsten Haitzler)[EMAIL PROTECTED] 裸好多 Tokyo, Japan (東京 日本) - This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ ___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel