[Spice-devel] Glib loop and scary corruption

[Frediano Ziglio]

Hi,
  I spend some time investigating in http://cgit.freedesktop.org/~fziglio/spice-server/commit/?h=refactory&id=48d5c2e74b6a866df2562c1aab8a1a3803bc788b,
one of the last problems of glib loop.
Beside I though that this patch also fix the symmetry between creation and destroying (stream is initialized during creation so usually is a good idea to deinitialize during destroying).
Looking at the backtrace you can see that stream is inside a event handler after being freed by red_channel_client_disconnect called trying to push items (red_channel_client_push).
My first idea was that glib called the event handler event after the watch was freed. This proved false after some testing.
So I decided to checkout more or less same version and look at the code.
Code was failing trying to retrieve header for a message, the push was called from red_channel_client_push handling a message of type SPICE_MSGC_ACK.
But messages are read and handled in a loop in red_peer_handle_incoming which is called by a single watcher!
So what's happen if:
- SPICE_MSGC_ACK is read
- handler is called with SPICE_MSGC_ACK
- handler try to push messages
- write fails as connection was closed
- red_channel_client_disconnect is called
- handler return to red_peer_handle_incoming
- red_peer_handle_incoming calls red_peer_receive to read next message header (NOTE: error handler in red_peer_handle_incoming is called only if message is not handled)
- red_peer_receive crash as rcc->stream is NULL!

If you add a red_channel_client_disconnect after red_channel_client_push in red_channel_client_handle_message you can see the same problem.
At this point. Why with Glib loop this happens more often? The reason is that the push is not called at last step of iteration so there is more probability to find items in red_channel_client_push (or at least the statistics on this change).

So to sum up:
- this issue is not a regression introduced by glib code;
- the only thing left to check for glib is this statistical stuff (already at good point for Jonathon work).

I'll post the patch based on master.

Frediano