Re: [Qemu-devel] [PATCH v2] thread-win32: fix GetThreadContext() permanently fails

[Paolo Bonzini]

On 01/07/2015 17:48, Zavadovsky Yan wrote:
> Ping.

Stefan, are you merging this?

Paolo

> Patchwork: http://patchwork.ozlabs.org/patch/488073/
> 
> v1
> discussion: 
> http://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg05791.html
> and patchworks: http://patchwork.ozlabs.org/patch/487438/
> http://patchwork.ozlabs.org/patch/487566/
> 
> On Wed, Jun 24, 2015 at 3:25 PM, Zavadovsky Yan
> <address@hidden <mailto:address@hidden>> wrote:
> 
>     Calling SuspendThread() is not enough to suspend Win32 thread.
>     We need to call GetThreadContext() after SuspendThread()
>     to make sure that OS have really suspended target thread.
>     But GetThreadContext() needs for THREAD_GET_CONTEXT
>     access right on thread object.
>     More info about this technique can be found here:
>     http://blogs.msdn.com/b/oldnewthing/archive/2015/02/05/10591215.aspx
> 
>     This patch adds THREAD_GET_CONTEXT to OpenThread() arguments
>     and change oddity 'while(GetThreadContext() == SUCCESS)' to
>     'if(GetThreadContext() == FAILED){exit(1);}'.
>     So this block of code will continue only after successful
>     grabbing of thread context(i.e. when thread is really suspended).
>     And halts otherwise with more verbose error message than previous.
> 
>     Signed-off-by: Zavadovsky Yan <address@hidden
>     <mailto:address@hidden>>
>     ---
>      cpus.c                   | 14 ++++++++------
>      util/qemu-thread-win32.c |  4 ++--
>      2 files changed, 10 insertions(+), 8 deletions(-)
> 
>     diff --git a/cpus.c b/cpus.c
>     index 4f0e54d..0df6a7d 100644
>     --- a/cpus.c
>     +++ b/cpus.c
>     @@ -1089,8 +1089,8 @@ static void qemu_cpu_kick_thread(CPUState *cpu)
>              CONTEXT tcgContext;
> 
>              if (SuspendThread(cpu->hThread) == (DWORD)-1) {
>     -            fprintf(stderr, "qemu:%s: GetLastError:%lu\n", __func__,
>     -                    GetLastError());
>     +            fprintf(stderr, "qemu:%s: SuspendThread
>     GetLastError:%lu\n",
>     +                    __func__, GetLastError());
>                  exit(1);
>              }
> 
>     @@ -1098,15 +1098,17 @@ static void qemu_cpu_kick_thread(CPUState *cpu)
>               * suspended until we can get the context.
>               */
>              tcgContext.ContextFlags = CONTEXT_CONTROL;
>     -        while (GetThreadContext(cpu->hThread, &tcgContext) != 0) {
>     -            continue;
>     +        if (GetThreadContext(cpu->hThread, &tcgContext) == 0) {
>     +            fprintf(stderr, "qemu:%s: GetThreadContext
>     GetLastError:%lu\n",
>     +                    __func__, GetLastError());
>     +            exit(1);
>              }
> 
>              cpu_signal(0);
> 
>              if (ResumeThread(cpu->hThread) == (DWORD)-1) {
>     -            fprintf(stderr, "qemu:%s: GetLastError:%lu\n", __func__,
>     -                    GetLastError());
>     +            fprintf(stderr, "qemu:%s: ResumeThread GetLastError:%lu\n",
>     +                    __func__, GetLastError());
>                  exit(1);
>              }
>          }
>     diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
>     index 406b52f..823eca1 100644
>     --- a/util/qemu-thread-win32.c
>     +++ b/util/qemu-thread-win32.c
>     @@ -406,8 +406,8 @@ HANDLE qemu_thread_get_handle(QemuThread *thread)
> 
>          EnterCriticalSection(&data->cs);
>          if (!data->exited) {
>     -        handle = OpenThread(SYNCHRONIZE | THREAD_SUSPEND_RESUME, FALSE,
>     -                            thread->tid);
>     +        handle = OpenThread(SYNCHRONIZE | THREAD_SUSPEND_RESUME |
>     THREAD_GET_CONTEXT,
>     +                            FALSE, thread->tid);
>          } else {
>              handle = NULL;
>          }
>     --
>     2.4.4.windows.2
> 
>