This fixes a thread race involving the monitor in monitor_qmp_event and monitor_qapi_event_emit .

Signed-off-by: Marc Morcos <marcmorcos@google.com>
---
 monitor/monitor.c | 11 ++++++++++-
 monitor/qmp.c     |  6 ++++--
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/monitor/monitor.c b/monitor/monitor.c
index c5a5d30877..4bfd72939d 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -338,6 +338,7 @@ static void monitor_qapi_event_emit(QAPIEvent event, QDict *qdict)
 {
     Monitor *mon;
     MonitorQMP *qmp_mon;
+    bool do_send = false;
 
     trace_monitor_protocol_event_emit(event, qdict);
     QTAILQ_FOREACH(mon, &mon_list, entry) {
@@ -346,7 +347,15 @@ static void monitor_qapi_event_emit(QAPIEvent event, QDict *qdict)
         }
 
         qmp_mon = container_of(mon, MonitorQMP, common);
-        if (qmp_mon->commands != &qmp_cap_negotiation_commands) {
+        do_send = false;
+
+        WITH_QEMU_LOCK_GUARD(&mon->mon_lock) {
+            if (qmp_mon->commands != &qmp_cap_negotiation_commands) {
+                do_send = true;
+            }
+        }
+
+        if (do_send) {
             qmp_send_response(qmp_mon, qdict);
         }
     }
diff --git a/monitor/qmp.c b/monitor/qmp.c
index cb99a12d94..e1419a9efa 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -462,8 +462,10 @@ static void monitor_qmp_event(void *opaque, QEMUChrEvent event)
 
     switch (event) {
     case CHR_EVENT_OPENED:
-        mon->commands = &qmp_cap_negotiation_commands;
-        monitor_qmp_caps_reset(mon);
+        WITH_QEMU_LOCK_GUARD(&mon->common.mon_lock) {
+            mon->commands = &qmp_cap_negotiation_commands;
+            monitor_qmp_caps_reset(mon);
+        }
         data = qmp_greeting(mon);
         qmp_send_response(mon, data);
         qobject_unref(data);
-- 
2.52.0.239.gd5f0c6e74e-goog

On 12/13/25 01:14, Marc Morcos wrote:
> @@ -346,7 +347,15 @@ static void monitor_qapi_event_emit(QAPIEvent event, QDict *qdict)
>           }
>   
>           qmp_mon = container_of(mon, MonitorQMP, common);
> -        if (qmp_mon->commands != &qmp_cap_negotiation_commands) {
> +        do_send = false;
> +
> +        WITH_QEMU_LOCK_GUARD(&mon->mon_lock) {
> +            if (qmp_mon->commands != &qmp_cap_negotiation_commands) {
> +                do_send = true;
> +            }
> +        }
> +
> +        if (do_send) {
>               qmp_send_response(qmp_mon, qdict);
>           }
>       }

We cannot use WITH_QEMU_LOCK_GUARD with "continue" or "break" inside, 
but we can use QEMU_LOCK_GUARD:

@@ -347,17 +346,13 @@ static void monitor_qapi_event_emit(QAPIEvent 
event, QDict *qdict)
          }

          qmp_mon = container_of(mon, MonitorQMP, common);
-        do_send = false;
-
-        WITH_QEMU_LOCK_GUARD(&mon->mon_lock) {
-            if (qmp_mon->commands != &qmp_cap_negotiation_commands) {
-                do_send = true;
+        {
+            QEMU_LOCK_GUARD(&mon->mon_lock);
+            if (qmp_mon->commands == &qmp_cap_negotiation_commands) {
+                continue;
              }
          }
-
-        if (do_send) {
-            qmp_send_response(qmp_mon, qdict);
-        }
+        qmp_send_response(qmp_mon, qdict);
      }
  }


Let me know if this is okay for you!

Paolo

Yes, that looks correct. Thanks!

- Marc

On Mon, Dec 15, 2025, 6:52 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 12/13/25 01:14, Marc Morcos wrote:
> > @@ -346,7 +347,15 @@ static void monitor_qapi_event_emit(QAPIEvent
> event, QDict *qdict)
> >           }
> >
> >           qmp_mon = container_of(mon, MonitorQMP, common);
> > -        if (qmp_mon->commands != &qmp_cap_negotiation_commands) {
> > +        do_send = false;
> > +
> > +        WITH_QEMU_LOCK_GUARD(&mon->mon_lock) {
> > +            if (qmp_mon->commands != &qmp_cap_negotiation_commands) {
> > +                do_send = true;
> > +            }
> > +        }
> > +
> > +        if (do_send) {
> >               qmp_send_response(qmp_mon, qdict);
> >           }
> >       }
>
> We cannot use WITH_QEMU_LOCK_GUARD with "continue" or "break" inside,
> but we can use QEMU_LOCK_GUARD:
>
> @@ -347,17 +346,13 @@ static void monitor_qapi_event_emit(QAPIEvent
> event, QDict *qdict)
>           }
>
>           qmp_mon = container_of(mon, MonitorQMP, common);
> -        do_send = false;
> -
> -        WITH_QEMU_LOCK_GUARD(&mon->mon_lock) {
> -            if (qmp_mon->commands != &qmp_cap_negotiation_commands) {
> -                do_send = true;
> +        {
> +            QEMU_LOCK_GUARD(&mon->mon_lock);
> +            if (qmp_mon->commands == &qmp_cap_negotiation_commands) {
> +                continue;
>               }
>           }
> -
> -        if (do_send) {
> -            qmp_send_response(qmp_mon, qdict);
> -        }
> +        qmp_send_response(qmp_mon, qdict);
>       }
>   }
>
>
> Let me know if this is okay for you!
>
> Paolo
>
>