[Spice-devel] [PATCH spice-gtk 11/15] channel-main: Make SpiceFileXferTask-s ref the channel

[Hans de Goede]

Hi,

First of all thanks for the review.

On 03/10/2013 03:28 PM, Marc-André Lureau wrote:
> On Sat, Mar 9, 2013 at 12:06 PM, Hans de Goede <hdegoede at redhat.com> wrote:
>> So that the channel sticks around while their callbacks are completing.
>
> I am not sure this is a good idea. Keeping channel references around
> will break migration code.

Hmm, nasty. Then fixing issues with the GFileInputStream callbacks possibly
returning after the channel has been finalized will need to be dealt with
in a different manner, something for me to think about.

Note that the flushing queue contains GAsyncSimpleResult-s with the
channel as source-object, so these have a reference to the channel too,
at least until they complete ...

> Have you checked with seamless & non-seamless migrations?

I don't expect much of a problem with non-seamless since the reset code
I added will cancel all pending xfers and thus free all references
(although with a slight delay if any operation is pending).

> The way session manages channels depends a lot on reference counting.
> It's not a good design, but no one fixed it yet. That makes me realize
> that referencing channels like this will probably be broken on
> migration, when the channel is replaced during non-seamless
> migrations.. I am afraid that will need some more work.

Hmm, so the channel is not replaced in the seamless case, right? So the
really troublesome scenario is the non-seamless, where the reset_agent
code will cancel the pending xfers (eventually) freeing the refs.

Regards,

Hans