Re: [Qemu-devel] linux-user: threading issue in the signal handler (Was: [PATCH] sh4-linux-user: fix multi-threading) regression.

[Peter Maydell]

On 3 April 2012 13:28,  <address@hidden> wrote:
> On Mon, Mar 26, 2012 at 07:23:58PM +0200, Peter Maydell wrote:
>> 2012/3/26 Cédric VINCENT <address@hidden>:
>> > the function cpu_restore_state() isn't expected to be called in user-mode,
>>
>> Is this really true? host_signal_handler() calls cpu_signal_handler()
>> calls handle_cpu_signala) calls cpu_restore_state() so hopefully
>> it's OK to be called in at least *some* situations...
>
> Actually it's not that OK, the code below [1] exposes a threading
> issue in this specific part of QEMU:
>
>    * the first thread makes invalid memory references, that way QEMU
>      reaches the given situation (host_signal_handler ->
>      cpu_signal_handler -> handle_cpu_signal -> cpu_restore_state ->
>      gen_intermediate_code) without acquiring "tb_lock".
>
>    * at the same time, the second thread executes code that wasn't
>      translated before, that way the TCG is invoked with "tb_lock"
>      acquired.  Note that in this example I used a self modifying
>      block just to simulate a not-yet-translated code.
>
> This example triggers an invalid memory reference and/or an assertion
> in the TCG part of QEMU.

Mmm, I rather suspected you had a larger problem than the one
you first encountered.

>> NB the whole tb_lock thing is broken anyway.
>
> Because of such bugs or are there other reasons?  Maybe we could add a
> simple sanity check in gen_intermediate_code() functions to be sure
> that "tb_lock" is acquired when they are called.

Eg https://bugs.launchpad.net/qemu/+bug/668799
Basically at the moment there are places where we try to update
the TB graph in functions called from signal handlers. This isn't
safe because we can't take a lock in the signal handler (we'd deadlock).
Instead we do no locking at all and if you make heavy use of threads
and signals we crash.

-- PMM