On Tue, Dec 19, 2017 at 04:45:40PM +0800, Peter Xu wrote:
> if (monitor_is_qmp(mon)) {
> - qemu_chr_fe_set_handlers(&mon->chr, monitor_can_read,
> monitor_qmp_read,
> - monitor_qmp_event, NULL, mon, NULL, true);
> qemu_chr_fe_set_echo(&mon->chr, true);
> json_message_parser_init(&mon->qmp.parser, handle_qmp_command);
> + qemu_chr_fe_set_handlers(&mon->chr, monitor_can_read,
> monitor_qmp_read,
> + monitor_qmp_event, NULL, mon, context,
> true);
> } else {
> + assert(!context); /* HMP does not support IOThreads */
> qemu_chr_fe_set_handlers(&mon->chr, monitor_can_read, monitor_read,
> monitor_event, NULL, mon, NULL, true);
> }
qemu_chr_fe_set_handlers() isn't thread-safe. It looks like
monitor_can_read()/monitor_qmp_read() can run in the IOThread at the
same time as monitor_qmp_event() runs in the main loop:
void qemu_chr_fe_set_handlers(CharBackend *b,
IOCanReadHandler *fd_can_read,
IOReadHandler *fd_read,
IOEventHandler *fd_event,
BackendChangeHandler *be_change,
void *opaque,
GMainContext *context,
bool set_open)
{
...
qemu_chr_be_update_read_handlers(s, context);
^-- The IOThread may invoke handler functions from now on!
<-- Everything below races with the IOThread!
if (set_open) {
qemu_chr_fe_set_open(b, fe_open);
}
if (fe_open) {
qemu_chr_fe_take_focus(b);
/* We're connecting to an already opened device, so let's make sure we
also get the open event */
if (s->be_open) {
qemu_chr_be_event(s, CHR_EVENT_OPENED);
}
}
if (CHARDEV_IS_MUX(s)) {
mux_chr_set_handlers(s, context);
}
}
It looks like chr_*() functions must be called from the event loop
thread that monitors the chardev. You can use aio_bh_schedule_oneshot()
or g_idle_source_new() to execute the last part of monitor_init() in the
GMainContext.
That way there is no race condition because qemu_chr_fe_set_handlers()
is called from within the event loop thread.
signature.asc
Description: PGP signature
