Re: [g-a-devel]exceptionally bad behavior ...

[Bill Haneman <bill haneman sun com>]

Michael!

Thanks a lot for the fix (which I am about to install/test), and the
very clear explanation (even I could understand it ;-)

I agree about the 'ev' reuse.  I hope that's just a matter of slogging
through the code, perhaps we can touch base on this at GUADEC and I can
prepare a patch to fix it sometime during the post-GUADEC week.  I will
report on whether the immediate fix solves the SEGVs we are seeing in
GOK.

regards,

Bill

On Fri, 2003-06-13 at 12:52, Michael Meeks wrote:
> Hi Bill,
> 
> 	Ok - so it turns out the oddness on exception is a _really_ silly
> at-spi/cspi bug ;-) [ you'll be pleased to know (and the ORB is doing
> the right thing ) ], but it throws up a more important / concerning bug
> IMHO - I'll see if it's feasible to knock up a patch for that shortly.
> 
> 	get_child_at_index does:
> 
>   retval = cspi_object_add (
>     Accessibility_Accessible_getChildAtIndex (CSPI_OBJREF (obj),
> 					      childIndex, cspi_ev ()));
> 
>   cspi_return_val_if_ev ("getChildAtIndex", NULL);
> 		
> 
> 	Which is fine: cspi_object_add does:
> 
>   else if (!cspi_check_ev ("pre method check: add"))
> 	retval = NULL;
> 
> 	which is also correct;
> 
> SPIBoolean
> cspi_check_ev (const char *error_string)
> {
>   CORBA_Environment *ev = cspi_ev ();
> 
>   if (ev->_major != CORBA_NO_EXCEPTION)
>     {
> 
> 	Which is _fine_ _BUT_ cspi_ev does:
> 
> CORBA_Environment *
> cspi_ev (void)
> {
>   CORBA_exception_init (&ev);
>   return &ev;
> }
> 
> 	ie. it wipes out the exception as soon as we check it. Thus AFAICS in
> cspi we have not been returning defined values from CORBA calls which
> fire exceptions for literally ages. [ it looks like this is my fault ].
> 
> 	So - an immediate fix to get it in is this:
> 
> Index: ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/at-spi/ChangeLog,v
> retrieving revision 1.317
> diff -u -p -u -r1.317 ChangeLog
> --- ChangeLog	11 Jun 2003 14:08:11 -0000	1.317
> +++ ChangeLog	13 Jun 2003 11:58:26 -0000
> @@ -1,3 +1,9 @@
> +2003-06-13  Michael Meeks  <michael ximian com>
> +
> +	* cspi/bonobo/cspi-bonobo.c (cspi_check_ev): use it.
> +
> +	* cspi/spi_main.c (cspi_peek_ev): impl.
> +
>  2003-06-11  Padraig O'Briain <padraig obriain sun com>
>  
>  	* cspi/spi-roletypes.h: Add role SPI_ROLE_AUTOCOMPLETE
> Index: cspi/spi-private.h
> ===================================================================
> RCS file: /cvs/gnome/at-spi/cspi/spi-private.h,v
> retrieving revision 1.14
> diff -u -p -u -r1.14 spi-private.h
> --- cspi/spi-private.h	11 Jun 2003 12:07:38 -0000	1.14
> +++ cspi/spi-private.h	13 Jun 2003 11:58:26 -0000
> @@ -68,6 +68,7 @@ struct _SPIException {
>  #define CSPI_OBJREF(a) (((Accessible *)(a))->objref)
>  
>  CORBA_Environment     *cspi_ev                (void);
> +CORBA_Environment     *cspi_peek_ev           (void);
>  SPIBoolean             cspi_exception         (void);
>  Accessibility_Registry cspi_registry          (void);
>  Accessible            *cspi_object_add        (CORBA_Object
> corba_object);
> Index: cspi/spi_main.c
> ===================================================================
> RCS file: /cvs/gnome/at-spi/cspi/spi_main.c,v
> retrieving revision 1.35
> diff -u -p -u -r1.35 spi_main.c
> --- cspi/spi_main.c	11 Jun 2003 12:07:39 -0000	1.35
> +++ cspi/spi_main.c	13 Jun 2003 11:58:27 -0000
> @@ -157,6 +157,12 @@ cspi_get_live_refs (void)
>  }
>  
>  CORBA_Environment *
> +cspi_peek_ev (void)
> +{
> +  return &ev;
> +}
> +
> +CORBA_Environment *
>  cspi_ev (void)
>  {
>    CORBA_exception_init (&ev);
> Index: cspi/bonobo/cspi-bonobo.c
> ===================================================================
> RCS file: /cvs/gnome/at-spi/cspi/bonobo/cspi-bonobo.c,v
> retrieving revision 1.9
> diff -u -p -u -r1.9 cspi-bonobo.c
> --- cspi/bonobo/cspi-bonobo.c	9 Jun 2003 22:32:50 -0000	1.9
> +++ cspi/bonobo/cspi-bonobo.c	13 Jun 2003 11:58:27 -0000
> @@ -41,7 +41,7 @@ cspi_release_unref (CORBA_Object object)
>  SPIBoolean
>  cspi_check_ev (const char *error_string)
>  {
> -  CORBA_Environment *ev = cspi_ev ();
> +  CORBA_Environment *ev = cspi_peek_ev ();
>  
>    if (ev->_major != CORBA_NO_EXCEPTION)
>      {
> 
> 
> 	This exposes a rather broader issue, which is that the continual re-use
> of that CORBA_Environment is quite badly broken;
> 
> 	Consider:
> 
> 	a) outgoing method with cspi_ev();
> 		b) incoming event
> 			c) outgoing method with cspi_ev returns
> 			   an exception - sets cspi_ev.
> 		even returns
> 	method (apparently) fails with exception - although it completed
> cleanly - resulting in a leak / bad result.
> 
> 	So - we really need to retro-fit the code en-masse to instantiate /
> initialize CORBA_Environments where we need them, rather than trying to
> re-use the system one.
> 
> 	Anyhow - I hope the fix will solve the immediate problem.
> 
> 	Regards,
> 
> 		Michael.
> 
> -- 
>  michael ximian com  <><, Pseudo Engineer, itinerant idiot
> 
> _______________________________________________
> Gnome-accessibility-devel mailing list
> Gnome-accessibility-devel gnome org
> http://mail.gnome.org/mailman/listinfo/gnome-accessibility-devel