qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Olga Krishtal
Subject: Re: [Qemu-devel] [PATCH v2] thread-win32: fix GetThreadContext() permanently fails
Date: Thu, 25 Jun 2015 12:11:06 +0300
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.7.0

On 24/06/15 15:25, 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 <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;
I would like to ask you if you have faced this situation in reality?
I have some doubts about changing the while() loop. According to the article - thread may mot be suspended immediately after code, due to the busy scheduler. If we do exit(1) just right after check of GetThreadContext(..) we can miss the situation when scheduler is too busy at the moment and just go down. From this point of view this while - is busy loop and gives the scheduler the opportunity to do its job. So, I am not sure about it.
+        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);
What was before the usage of this flag? I mean the behavior? As I can see it worked even without this flag.
+        handle = OpenThread(SYNCHRONIZE | THREAD_SUSPEND_RESUME | 
THREAD_GET_CONTEXT,
+                            FALSE, thread->tid);
      } else {
          handle = NULL;
      }




reply via email to

[Prev in Thread] Current Thread [Next in Thread]