Re: [Qemu-devel] [PATCH v2] monitor: temporary fix for dead-lock on event recursion

[Eric Blake]

On 07/31/2018 10:01 AM, Marc-André Lureau wrote:
> With a Spice port chardev, it is possible to reenter
> monitor_qapi_event_queue() (when the client disconnects for
> example). This will dead-lock on monitor_lock.
>
> Instead, use some TLS variables to check for recursion and queue the
> events.

> diff --git a/monitor.c b/monitor.c
> index d8d8211ae4..4d9c1873bc 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -633,7 +633,7 @@ static void monitor_qapi_event_handler(void *opaque);
>    * applying any rate limiting if required.
>    */
>   static void
> -monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
> +monitor_qapi_event_queue_no_recurse(QAPIEvent event, QDict *qdict)
>   {
>       MonitorQAPIEventConf *evconf;
>       MonitorQAPIEventState *evstate;
> @@ -688,6 +688,48 @@ monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, 
> Error **errp)
>       qemu_mutex_unlock(&monitor_lock);
>   }
>   
> +static void
> +monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
> +{
> +    /*
> +     * monitor_qapi_event_queue_no_recurse() is not reentrant: it
> +     * would deadlock on monitor_lock.  Work around by queueing
> +     * events in thread-local storage.
> +     * TODO: remove this, make it re-enter safe.
> +     */
> +    static __thread bool reentered;
> +    typedef struct MonitorQapiEvent {
> +        QAPIEvent event;
> +        QDict *qdict;
> +        QSIMPLEQ_ENTRY(MonitorQapiEvent) entry;
> +    } MonitorQapiEvent;
> +    MonitorQapiEvent *ev;
> +    static __thread QSIMPLEQ_HEAD(, MonitorQapiEvent) event_queue;
> +
> +    if (!reentered) {
> +        QSIMPLEQ_INIT(&event_queue);
> +    }

Is it safe to call QSIMPLEQ_INIT() on an already-initialized but empty 
queue?

> +
> +    ev = g_new(MonitorQapiEvent, 1);
> +    ev->qdict = qobject_ref(qdict);
> +    ev->event = event;
> +    QSIMPLEQ_INSERT_TAIL(&event_queue, ev, entry);
> +    if (reentered) {
> +        return;
> +    }
> +
> +    reentered = true;
> +
> +    while ((ev = QSIMPLEQ_FIRST(&event_queue)) != NULL) {
> +        QSIMPLEQ_REMOVE_HEAD(&event_queue, entry);
> +        monitor_qapi_event_queue_no_recurse(ev->event, ev->qdict);
> +        qobject_unref(ev->qdict);
> +        g_free(ev);
> +    }
> +
> +    reentered = false;
> +}
> +
>   /*
>    * This function runs evconf->rate ns after sending a throttled
>    * event.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org