Peter Xu <pet...@redhat.com> writes: > After the Out-Of-Band work, the monitor iothread may be accessing the > cur_mon as well (via monitor_qmp_dispatch_one()). Let's convert the > cur_mon variable to be a per-thread variable to make sure there won't be > a race between threads when accessing the variable.
Hmm... why hasn't the OOB work created such a race already? A monitor reads, parses, dispatches and executes commands, formats and sends replies. Before OOB, all of that ran in the main thread. Any access of cur_mon should therefore be from the main thread. No races. OOB moves read, parse, format and send to an I/O thread. Dispatch and execute remain in the main thread. *Except* for commands executed OOB, dispatch and execute move to the I/O thread, too. Why is this not racy? I guess it relies on careful non-use of cur_mon in any part that may now execute in the I/O thread. Scary... Should this go into 3.0 to reduce the risk of bugs? > Note that thread variables are not initialized to a valid value when new > thread is created. However for our case we don't need to set it up, > since the cur_mon variable is only used in such a pattern: > > old_mon = cur_mon; > cur_mon = xxx; > (do something, read cur_mon if necessary in the stack) > cur_mon = old_mon; > > It plays a role as stack variable, so no need to be initialized at all. > We only need to make sure the variable won't be changed unexpectedly by > other threads. > > Reviewed-by: Eric Blake <ebl...@redhat.com> > Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com> > Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> > [peterx: touch up commit message a bit] > Signed-off-by: Peter Xu <pet...@redhat.com>
