Re: [gdbserver/win32] (4/11) New interrupting method

[Pedro Alves <pedro_alves@portugalmail.pt>]

Lerele wrote:
> I think this patch breaks interrupt functionality.
> The function suspend_one_thread in the new patch does not seem to 
> retreive the thread context and sets the thread suspend count to 1, so 
> the next call to thread_rec that should happen in gdbserver will get 
> incorrect thread context.
> 
> My interrupt patch I sent initially used thread_rec to pause the child 
> threads, so get context should work here.
> I think it's more correct anyway to use thread_rec everywhere so that 
> SuspendThread is centralized in one unique function.
> 
> Is this correct, or is there something I may have slipped reading your 
> patch?
> 

You slipped reading the previous patch in the series  :-)

The previous patch in the series [1] has this hunk:

@@ -105,10 +105,19 @@ thread_rec (DWORD id, int get_context)
      return NULL;

    th = inferior_target_data (thread);
-  if (!th->suspend_count && get_context)
+  if (get_context && th->context.ContextFlags == 0)
      {
-      if (id != current_event.dwThreadId)
-       th->suspend_count = SuspendThread (th->h) + 1;
+      if (!th->suspended)
+       {
+         if (SuspendThread (th->h) == (DWORD) -1)
+           {
+             DWORD err = GetLastError ();
+             OUTMSG (("warning: SuspendThread failed in thread_rec, "
+                      "(error %d): %s\n", (int) err, strwinerror (err)));
+           }
+         else
+           th->suspended = 1;
+       }

        (*the_low_target.get_thread_context) (th, &current_event);
      }

Which means that thread_rec will fetch the register context
if it hasn't already (th->context.ContextFlags == 0),
not if the thread wasn't suspended, which makes more sense:
"If you want the registers, and you don't have them already",
as opposed to:
"If you want the registers and the thread is not suspended".

[1] - [gdbserver/win32] (3/11) Fix suspend count handling
       http://sourceware.org/ml/gdb-patches/2007-11/msg00216.html

-- 
Pedro Alves