From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15047 invoked by alias); 21 Nov 2007 11:25:59 -0000 Received: (qmail 15016 invoked by uid 22791); 21 Nov 2007 11:25:56 -0000 X-Spam-Check-By: sourceware.org Received: from ics.u-strasbg.fr (HELO ics.u-strasbg.fr) (130.79.112.250) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 21 Nov 2007 11:25:43 +0000 Received: from ICSMULLER (laocoon.u-strasbg.fr [130.79.112.72]) by ics.u-strasbg.fr (Postfix) with ESMTP id 093C718702B; Wed, 21 Nov 2007 12:30:15 +0100 (CET) From: "Pierre Muller" To: "'Pedro Alves'" , References: <47437D3A.3000107@portugalmail.pt> In-Reply-To: <47437D3A.3000107@portugalmail.pt> Subject: RE: [win32] Fix suspend count handling Date: Wed, 21 Nov 2007 11:25:00 -0000 Message-ID: <000001c82c31$4a57b220$df071660$@u-strasbg.fr> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Office Outlook 12.0 Content-Language: en-us Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-11/txt/msg00393.txt.bz2 Hi Pedro, I agree with you that the ResumeThread should be called only once, even if the suspend_count is > 1, but when I tested your test application, I obtained 4 as a result with the current gdb head. This is because in fact the thread suspend_count are only restored after ContinueDebugEvent is called. Thus the debugged event can possibly called ResumeThread before gdb does so, thus obtaining a Thread count of 4 instead of 0... The first answer that comes to mind would be to called ResumeThread before calling ContinueDebugEvent, but that would mean that other threads would be restarted before the thread that caused the debug event, which is probably also not good. I would propose to do it this way: -for threads that where already suspended, and for which gdb increased the suspend count, we should restore the suspend count before calling ContinueDebugEvent, while for the thread that were not suspended before, we should resume them after the call to ContinueDebugEvent. This means that we should keep the suspend_count variable as an int and loop once before ContinueDebugEvent call (restoring the suspend count of debuggee suspended threads, identified by the fact that their suspend_count is > 1, calling ResumeThread only once, as you suggest in your patch), and loop again after the call to resume all other threads that were running at the time of the debug event (those which have a suspend_count of 1). Pierre > -----Original Message----- > From: gdb-patches-owner@sourceware.org [mailto:gdb-patches- > owner@sourceware.org] On Behalf Of Pedro Alves > Sent: Wednesday, November 21, 2007 1:35 AM > To: gdb-patches@sourceware.org > Subject: [win32] Fix suspend count handling > > Hi, > > This patch fixes a problem that was originally reported against > gdbserver/win32, which is also present on a native win32 debugger. > > The gdbserver patch is here: > [gdbserver/win32] (3/11) Fix suspend count handling > http://sourceware.org/ml/gdb-patches/2007-11/msg00216.html > > Let me re-describe the problem: > > The current suspend_count handling breaks applications that use > SuspendThread on their own threads. The SuspendThread function > suspends a thread and returns its previous suspend count. > If a thread was already suspended by the debuggee, say, 3 times, gdb > will do one SuspendThread call more (in thread_rec), bringing the > suspend_count to 4. Later, when the inferior is being resumed, gdb > will call ResumeThread suspend_count times, in this case 4 times, which > will leave a thread running, while the debuggee wanted it to be > suspended (with a suspend count of 3). > > This patch fixes it by turning the suspend_count counter into a > suspended flag that records if gdb itself has suspended the thread. > Gdb will then only call SuspendThread on a thread if it hasn't already, > and will only call Resume thread if it has called SuspendThread itself. > > Here is a test that shows the problem: > > >cat ~/suspendfix/main.c > #include > > HANDLE started; > HANDLE stop; > > DWORD WINAPI > thread_start (void *arg) > { > SetEvent (started); > WaitForSingleObject (stop, INFINITE); > return 0; > } > > int > main (int argc, char **argv) > { > int i; > DWORD suspend_count; > started = CreateEvent (NULL, TRUE, FALSE, NULL); > stop = CreateEvent (NULL, TRUE, FALSE, NULL); > > HANDLE h = CreateThread (NULL, 0, thread_start, NULL, > 0, NULL); > > WaitForSingleObject (started, INFINITE); > > for (i = 0; i < 3; i++) > if (SuspendThread (h) == (DWORD) -1) > { > printf ("SuspendThreadFailed\n"); > return 1; > } > > suspend_count = ResumeThread (h); /* set breakpoint here */ > > printf ("%lu\n", suspend_count); /* should be 3 */ > > while ((suspend_count = ResumeThread (h)) != 0 > && suspend_count != -1) > ; > SetEvent (stop); > WaitForSingleObject (h, INFINITE); > CloseHandle (h); > CloseHandle (started); > CloseHandle (stop); > return 0; > } > > Set a breakpoint where it reads "set breakpoint here", switch to the > other thread with "thread 2", effectivelly forcing a thread_rec call, > which calls SuspendThread, and sets suspend_count. Go back to thread > 1, and advance to the printf call. The suspend_count will be 0, > instead of the expected 3. > > I've asked this on the gdbserver mail, but let me re-ask it, because I > haven't done that work yet :-): > I have yet to turn this into a proper gdb test. Is that > wanted? Where shall I put it? gdb.base? gdb.thread? other? > > No regressions on i686-pc-cygwin. > > Cheers, > Pedro Alves > >