From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12657 invoked by alias); 22 Nov 2007 09:19:20 -0000 Received: (qmail 12643 invoked by uid 22791); 22 Nov 2007 09:19:19 -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; Thu, 22 Nov 2007 09:19:11 +0000 Received: from ICSMULLER (laocoon.u-strasbg.fr [130.79.112.72]) by ics.u-strasbg.fr (Postfix) with ESMTP id F0EFD18701A; Thu, 22 Nov 2007 10:23:41 +0100 (CET) From: "Pierre Muller" To: "'Pedro Alves'" Cc: References: <47437D3A.3000107@portugalmail.pt> <000001c82c31$4a57b220$df071660$@u-strasbg.fr> <4053daab0711210543w4b241e1ek2371e887f3c4f7d2@mail.gmail.com> <000401c82c48$a450df10$ecf29d30$@u-strasbg.fr> <4053daab0711210708o607018b9n8b63147a8498a207@mail.gmail.com> <4053daab0711211019r15f3a862g677080b65b4d8e71@mail.gmail.com> <4744BCCE.60705@portugalmail.pt> In-Reply-To: <4744BCCE.60705@portugalmail.pt> Subject: RE: [win32] Fix suspend count handling Date: Thu, 22 Nov 2007 09:19:00 -0000 Message-ID: <002401c82ce8$c82f7f90$588e7eb0$@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/msg00406.txt.bz2 I checked again the new patch and the testsuite results are exactly the same as I already reported for the hardware watchpoint fix alone. I think that this patch is good, but only Christopher can approve it. Pierre > -----Original Message----- > From: gdb-patches-owner@sourceware.org [mailto:gdb-patches- > owner@sourceware.org] On Behalf Of Pedro Alves > Sent: Thursday, November 22, 2007 12:19 AM > To: Pierre Muller > Cc: gdb-patches@sourceware.org > Subject: Re: [win32] Fix suspend count handling > > Pedro Alves wrote: > > Pedro Alves wrote: > >> On Nov 21, 2007 2:13 PM, Pierre Muller wrote: > >> That's not what I see here. Can you show me a run where you get 4 > >> only this patch applied? > >> > > > > I did try that, but posted a log of not doing it :-). > > > > I've just tried about 30 times, and only once I did see a 4 coming > out > > ... oh, well, one of those things. > > > > OK. Back at my home laptop, I can reproduce that with no problems. > Let me clarify what the 4 problem really is. > It's a race between gdb and the inferior. > > Take this slightly changed test case. The only difference to the > original version is the extra Sleep call. > > #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; > } > > Sleep (300); > > 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; > } > > If you do the "break at ...", "run", "thread 3", "continue" > sequence, and "..." is the "Sleep" line, you'll get 3, but if you put > the break at the /* set breakpoint here */ line, you'll get 4 (if > you're (un)lucky). > > The race happens due to the fact that gdb is doing something similar to > this: > > win32_continue() > { > ContinueDebugEvent (...); /* Resumes all non suspended > threads of the process. */ > > /* At this point, gdb is running concurrently with > the inferior threads that were not suspended - which > included the main thread of the testcase. */ > foreach t in threads do > if t is suspended > ResumeThread t > fi > done > } > > If you break at the Sleep call, when we resume, gdb will have a bit of > time to call ResumeThread on the suspended thread of the testcase. If > you instead break at the ResumeThread line, you'll have a good chance > that the inferior wins the race, hence the "4" result (remember that > ResumeThread returns the previous suspend count). > If we put something like this after the ResumeThread call: > > (...) > suspend_count = ResumeThread (h); /* set breakpoint here */ > > + Sleep (300); > + SuspendThread (h); > + suspend_count = ResumeThread (h); > > printf ("%lu\n", suspend_count); /* should be 3 */ > (...) > > ... you'll see that eventually gdb will bring the suspend count back to > 3. (A SuspendThread, ResumeThread pair is the way to get at the > suspend count.) > > > Since the watchpoint patch should fix this, what shall I do? Shall I > > merge the two and resubmit, or leave it at that ? They've already > > been tested together without regressions. > > > > Here is the merge from the patch I posted at the start of the thread > with this patch: > [win32] Fix watchpoint support > http://sourceware.org/ml/gdb-patches/2007-11/msg00390.html > > This patch fixes both the suspend_count > handling, and the watchpoint support. > > Thanks Pierre, for looking at it. > > OK ? > > -- > Pedro Alves