Re: [Qemu-devel] [RFC PATCH] char-socket: hold chr_write_lock during tcp_chr_free_connection()

[Alberto Garcia]

On Wed 06 Feb 2019 02:00:03 PM CET, Paolo Bonzini wrote:
> On 06/02/19 13:43, Alberto Garcia wrote:
>>          }
>> @@ -449,7 +451,9 @@ static void tcp_chr_disconnect(Chardev *chr)
>>      SocketChardev *s = SOCKET_CHARDEV(chr);
>>      bool emit_close = s->connected;
>>  
>> +    qemu_mutex_lock(&chr->chr_write_lock);
>>      tcp_chr_free_connection(chr);
>> +    qemu_mutex_unlock(&chr->chr_write_lock);
>>  
>>      if (s->listener) {
>>          qio_net_listener_set_client_func_full(s->listener, tcp_chr_accept,
>
> Should operations on the listener also be protected?  I think that apart
> from emitting the closed event itself everything in this function should
> be protected by the lock.   The closed event should be pushed to the
> GMainContext using an idle source (perhaps it's worth writing a wrapper
> qemu_idle_add that takes a GMainContext, as the same idiom is already
> present in pty_chr_state and qio_task_thread_worker).

Now that you mention this it seems that we are leaking the GSources in
those two cases? GLib's g_idle_add_full() implementation does unref the
source after attaching it to the GMainContext.

   https://gitlab.gnome.org/GNOME/glib/blob/master/glib/gmain.c#L5684

Berto