[E-devel] Evas keydown segfault

2007-04-08 Thread Christopher Michael

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

2007-04-18 Thread Brian Mattern
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

2007-04-19 Thread Brian Mattern
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

2007-04-19 Thread [EMAIL PROTECTED]

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

2007-04-19 Thread Sebastian Dransfeld

[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

2007-04-19 Thread [EMAIL PROTECTED]
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

2007-04-24 Thread The Rasterman
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

2007-04-24 Thread The Rasterman
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

2007-04-29 Thread The Rasterman
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