From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40943) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZALCF-00067P-Fz for qemu-devel@nongnu.org; Wed, 01 Jul 2015 12:49:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZALCC-0004v6-9X for qemu-devel@nongnu.org; Wed, 01 Jul 2015 12:49:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42187) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZALCC-0004rt-0o for qemu-devel@nongnu.org; Wed, 01 Jul 2015 12:49:12 -0400 References: <1435148753-4476-1-git-send-email-zavadovsky.yan@gmail.com> From: Paolo Bonzini Message-ID: <55941A03.8040803@redhat.com> Date: Wed, 1 Jul 2015 18:49:07 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] thread-win32: fix GetThreadContext() permanently fails List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zavadovsky Yan , qemu-devel@nongnu.org Cc: Olivier Hainque , Peter Maydell , Fabien Chouteau , Stefan Weil 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 > > 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 > > --- > 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 > >