Peter Xu <pet...@redhat.com> writes: > On Thu, Jul 19, 2018 at 02:14:56PM +0200, Markus Armbruster wrote: [...] >> Here's my try: >> >> monitor: Fix unsafe sharing of @cur_mon among threads >> >> @cur_mon is null unless the main thread is running monitor code, either >> HMP code within monitor_read(), or QMP code within >> monitor_qmp_dispatch(). >> >> Use of @cur_mon outside the main thread is therefore unsafe. >> >> Most of its uses are in monitor command handlers. These run in the main >> thread. >> >> However, there are also uses hiding elsewhere, such as in >> error_vprintf(), and thus error_report(), making these functions unsafe >> outside the main thread. No such unsafe uses are known at this time. >> Regardless, this is an unnecessary trap. It's an ancient trap, though. >> >> More recently, commit cf869d53172 "qmp: support out-of-band (oob) >> execution" spiced things up: the monitor I/O thread assigns to @cur_mon >> when executing commands out-of-band. Having two threads save, set and >> restore @cur_mon without synchronization is definitely unsafe. We can >> end up with @cur_mon null while the main thread runs monitor code, or >> non-null while it runs non-monitor code. >> >> We could fix this by making the I/O thread not mess with @cur_mon, but >> that would leave the trap armed and ready. >> >> Instead, make @cur_mon thread-local. It's now reliably null unless the >> thread is running monitor code. > > Looks good to me.
With that commit message: Reviewed-by: Markus Armbruster <arm...@redhat.com>
