Re: [gtk-vnc-devel] Segfault

[Anthony Liguori <anthony codemonkey ws>]

Daniel P. Berrange wrote:
> On Wed, Oct 03, 2007 at 03:49:26PM -0300, Jonh Wendell wrote:
>   
> > Em Qua, 2007-10-03 às 19:29 +0100, Daniel P. Berrange escreveu:
> >     
> > > On Wed, Oct 03, 2007 at 03:00:23PM -0300, Jonh Wendell wrote:
> > >       
> > > > Hi, folks.
> > > >
> > > > I'm getting a segmentation fault in vinagre, related to gtk-vnc:
> > > >
> > > > -I connect into 200.100.100.246, type my password, the connection is OK;
> > > > -I connect into 200.100.100.191, type my password, and vinagre crashes.
> > > >         
> > > So, you're using 2 separate connections at the same time?
> > >       
> > Yep, vinagre has support to multiple simultaneous connections, via tabs.
> >     
>
> So as I mentioned on IRC this turned out to be the result of several factors
> conspiring. The ultimate result though is that the 2 coroutines for each
> active VNC connection end up yielding to each other forever, never going
> back to the main system coroutine. Eventually one of them yields with 
> control going back into the GLib main loop with invalid context. SEGV
> follows shortly.
>
> It is a tricky situation to understand, so I'll walk through the steps
>
>  1. Start out in system coroutine, S
>  2. Connect to first server, transitioning to coroutine A. A's caller is
>     set to S
>  3. A blocks on I/O, so it yields back to its caller S. S's caller is now
>     set to A.
>  4. When I/O is ready, S yield explicitly to A. 
>
> Steps 3 & 4 continue ad-inifinitum as data comes in from the server. Notice,
> however, step in step 3, we unneccessarily set S's caller to A. This is not
> neccessary, because the GLib I/O callback has a direct reference to A, and
> will explicitly yield back to A, so caller of S is never used.
>
> Now, consider that we open a second connection..
>
>  1. Start out in system coroutine, S
>  2. Connect to second server, transitioning to coroutine B. B's caller is
>     set to S
>  3. B blocks on I/O, so it yields back to its caller S. S's caller is now
>     set to B.
>  4. When I/O is ready, S yield explicitly to B.
>      ...repeat...
>  5. B gets to point where it needs to get a password to authenticate, so 
>     we fire auth callback to the application.
>  6. Vinagre sets up its auth dialog box and runs gtk_dialog_run()
>  7. gtk_dialog_run() spawns a nested main loop
>  8. I/O is now ready for coroutine A again, B yields to A, setting A's
>     caller to be B.
>  9. A blocks on I/O again, so yields back to its caller B. B's caller
>     is now set to A.
>
> At this point A's caller is B, and B's caller is A. There is now no way we
> can ever yield back to the system co-routine S.
>
> It bounces back & forth but soon goes kaboom.
>   

I think I'm following your logic here and I agree with your assessment.  
The switch from system => leader made the patch a little less clear but 
I'm pretty confident that that was the correct fix.

Thanks!

Regards,

Anthony Liguori

> In considering this I observe:
>
>  - The yield() call swaps to the caller.
>  - The yieldto() call swpas to the specified coroutine.
>  - The system co-routine must never call 'yield()', since it does not
>    have anyone to yield to.
>
> Following on from this:
>
>  - The yield() call must use the caller, and then set it to NULL
>  - The yieldto() call must set the caller and not use it.
>  - It is incorect to call yieldto() if the destination has a caller
>    already set - this indicates re-entrancy
>  - It is incorrect to call yield() if you don't have a caller set
>
> So the attach patch makes sure yeild() sets the caller to NULL. It also calls
> an immediate abort() if either of the last two conditions occur. This ensures
> we get a stack trace at the point the error actually occurs, rather than a
> SEGV some random time later.
>
> NB I had to rename system to leader, because when I include stdlib.h the
> system variable clashes with the system() fnuction.
>
> With this patch applied I can connect to multiple servers, and even re-enter
> the mainloop from within a co-routine without messing up the coroutine caller
> chain.
>
> Dan
>   
> ------------------------------------------------------------------------
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc.
> Still grepping through log files to find problems?  Stop.
> Now Search log events and configuration files using AJAX and a browser.
> Download your FREE copy of Splunk now >> http://get.splunk.com/
> ------------------------------------------------------------------------
>
> _______________________________________________
> Gtk-vnc-devel mailing list
> Gtk-vnc-devel lists sourceforge net
> https://lists.sourceforge.net/lists/listinfo/gtk-vnc-devel