[Spice-devel] [spice-gtk 3/5] session: don't track open_host_idle source id

[Marc-André Lureau]

On Wed, Feb 5, 2014 at 9:36 AM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> On Tue, Feb 04, 2014 at 03:22:47PM +0100, Marc-André Lureau wrote:
>> On Tue, Feb 4, 2014 at 2:48 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
>> > On Wed, Jan 22, 2014 at 07:26:49PM +0100, Marc-André Lureau wrote:
>> >> From: Marc-André Lureau <marcandre.lureau at redhat.com>
>> >>
>> >> In all cases, when the coroutine is resumed, the idle source has been
>> >> removed.
>> >
>> > Are you sure about this? I see some coroutine_yieldto() in error cases in
>> > open_host_idle_cb (but I always get the coroutine code wrong).
>> >
>>
>> but the source is removed there anyway
>
> As I understand it, even if the coroutine is resumed through a
> coroutine_yieldto() during the idle execution, it will eventually be
> removed through a return G_SOURCE_REMOVE, is this what you mean ?

yes

>>
>> > Also, the commit which added this code was saying that we could get back to
>> > the coroutine before the idle runs:
>> >
>> > commit 5ed7f06e7c7a9fa9c6b0411debd43793335068ec
>> > Author: Marc-André Lureau <marcandre.lureau at redhat.com>
>> > Date:   Sun Dec 23 20:56:09 2012 +0100
>> >
>> >     Clean-up idle handler when leaving the open_host_idle()
>> >
>> >     An explicit yield back to the channel coroutine when the idle function
>> >     is still pending will leave it in the background, referencing objects
>> >     that may no longer exist. Make sure we remove it when
>> >     channel_open_host() is resumed.
>> >
>> > Is this no longer valid?
>>
>> Yes, I think this I was over-zealous. You shouldn't resume coroutines
>> at will, depending on their state it might be a wrong thing to do. If
>> there is a pending resume / return in an idle source, then it's wrong
>> to resume outside.
>
> This commit log was about scheduling an idle, then switching back to the
> main context, and then switching back to the coroutine context before
> the idle even starts running. I think you are talking about something
> different.
>
> Christophe



-- 
Marc-André Lureau