From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7666 invoked by alias); 26 Nov 2007 09:21:25 -0000 Received: (qmail 7655 invoked by uid 22791); 26 Nov 2007 09:21:24 -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; Mon, 26 Nov 2007 09:21:17 +0000 Received: from ICSMULLER (laocoon.u-strasbg.fr [130.79.112.72]) by ics.u-strasbg.fr (Postfix) with ESMTP id A466F18701F for ; Mon, 26 Nov 2007 10:25:44 +0100 (CET) From: "Pierre Muller" To: References: <002801c82d06$21bdf510$6539df30$@u-strasbg.fr> <4053daab0711220456q46cca9b4m3714c35bcc805518@mail.gmail.com> <002c01c82d0f$8c789050$a569b0f0$@u-strasbg.fr> <4053daab0711220637h2bc01450ra45a19f4013fd44d@mail.gmail.com> <20071123011754.GB31180@ednor.casa.cgf.cx> <006201c82dec$de431850$9ac948f0$@u-strasbg.fr> <20071124054405.GB4214@ednor.casa.cgf.cx> In-Reply-To: <20071124054405.GB4214@ednor.casa.cgf.cx> Subject: [RFA 2] gdb/win32-nat.c: do not call CloseHandle on process and thread handles Date: Mon, 26 Nov 2007 09:21:00 -0000 Message-ID: <000101c8300d$c1c85960$45590c20$@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/msg00481.txt.bz2 > -----Original Message----- > From: gdb-patches-owner@sourceware.org [mailto:gdb-patches- > owner@sourceware.org] On Behalf Of Christopher Faylor > Sent: Saturday, November 24, 2007 6:44 AM > To: gdb-patches@sourceware.org > Subject: Re: [RFA] gdb/win32-nat.c: do not call CloseHandle on process > and thread handles > > On Fri, Nov 23, 2007 at 05:21:14PM +0100, Pierre Muller wrote: > > Following Christopher and Pedro's comments, > >I added a new variable called fake_create_used > >to record if the current_process_handle is > >coming from OpenProcess called in fake_create_process, > >in which case we should close the handle, or > >coming from the CREATE_PROCESS_DEBUG_EVENT, > >which is closed by the system. > > > >Pierre > > > >ChangeLog entry: > > > >2007-11-23 Pierre Muller > > > > *win32-nat.c (fake_create_used): New static variable. > > (win32_init_thread_list): Remove call to CloseHandle for thread. > > (win32_delete_thread): Ditto. > > (fake_create_process): Set fake_create_used if OpenProcess call > > is successful. > > (get_win32_debug_event): Do not close process handle. > > (do_initial_win32_stuff): Set fake_create_used to zero. > > (win32_mourn_inferior): Call CloseHandle for > current_process_handle > > if fake_create_process is set. > > (win32_kill_inferior): Do not close process and main_thread > handles. > > I'd prefer if you called the flag variable "open_process_used" since I > think that's more descriptive. I changed this variable name. > Also, rather than using printf_unfiltered, shouldn't you be using > error, > like the rest of the code? I don't really know where error would bring us back, that is why I did not dare to use it, but I also changed that part according to your suggestion. I tried to manually trigger the error code, and got an error indeed, but I could still do a 'continue' after gdb which is probably not right either... > And, the GNU C coding standard seems to imply that braces around single > line statements after an `if' are not required. Most of the code in > win32-nat.c follows this convention so I would appreciate if you would > eliminate this use of braces where appropriate. > > Other than that, the patch looks good. > > cgf adapted ChangeLog entry: 2007-11-26 Pierre Muller *win32-nat.c (open_process_used): New static variable. (win32_init_thread_list): Remove call to CloseHandle for thread. (win32_delete_thread): Ditto. (fake_create_process): Set open_process_used if OpenProcess call is successful. (get_win32_debug_event): Do not close process handle. (do_initial_win32_stuff): Set open_process_used to zero. (win32_mourn_inferior): Call CloseHandle for current_process_handle if open_process_used is set. (win32_kill_inferior): Do not close process and main_thread handles. Index: gdb/win32-nat.c =================================================================== RCS file: /cvs/src/src/gdb/win32-nat.c,v retrieving revision 1.140 diff -u -p -r1.140 win32-nat.c --- gdb/win32-nat.c 24 Nov 2007 12:13:28 -0000 1.140 +++ gdb/win32-nat.c 26 Nov 2007 08:45:33 -0000 @@ -141,6 +141,7 @@ static DWORD main_thread_id; /* Thread static int exception_count = 0; static int event_count = 0; static int saw_create; +static int open_process_used = 0; /* User options. */ static int new_console = 0; @@ -325,7 +326,6 @@ win32_init_thread_list (void) { thread_info *here = th->next; th->next = here->next; - (void) CloseHandle (here->h); xfree (here); } thread_head.next = NULL; @@ -350,7 +350,6 @@ win32_delete_thread (DWORD id) { thread_info *here = th->next; th->next = here->next; - CloseHandle (here->h); xfree (here); } } @@ -1171,6 +1170,14 @@ fake_create_process (void) { current_process_handle = OpenProcess (PROCESS_ALL_ACCESS, FALSE, current_event.dwProcessId); + if (current_process_handle != NULL) + open_process_used = 1; + else + { + error (_("OpenProcess call failed, GetLastError = %lud\n"), + GetLastError ()); + /* We can not debug anything in that case. */ + } main_thread_id = current_event.dwThreadId; current_thread = win32_add_thread (main_thread_id, current_event.u.CreateThread.hThread); @@ -1299,7 +1306,8 @@ get_win32_debug_event (int pid, struct t thread event. Caused when attached process does not have a main thread. */ retval = ourstatus->value.related_pid = fake_create_process (); - saw_create++; + if (retval) + saw_create++; } break; } @@ -1333,7 +1341,6 @@ get_win32_debug_event (int pid, struct t CloseHandle (current_event.u.CreateProcessInfo.hFile); if (++saw_create != 1) { - CloseHandle (current_event.u.CreateProcessInfo.hProcess); break; } @@ -1356,7 +1363,6 @@ get_win32_debug_event (int pid, struct t break; ourstatus->kind = TARGET_WAITKIND_EXITED; ourstatus->value.integer = current_event.u.ExitProcess.dwExitCode; - CloseHandle (current_process_handle); retval = main_thread_id; break; @@ -1488,6 +1494,7 @@ do_initial_win32_stuff (DWORD pid) last_sig = TARGET_SIGNAL_0; event_count = 0; exception_count = 0; + open_process_used = 0; debug_registers_changed = 0; debug_registers_used = 0; for (i = 0; i < sizeof (dr) / sizeof (dr[0]); i++) @@ -1886,9 +1893,6 @@ win32_create_inferior (char *exec_file, error (_("Error creating process %s, (error %d)."), exec_file, (unsigned) GetLastError ()); - CloseHandle (pi.hThread); - CloseHandle (pi.hProcess); - if (useshell && shell[0] != '\0') saw_create = -1; else @@ -1904,6 +1908,11 @@ win32_mourn_inferior (void) { (void) win32_continue (DBG_CONTINUE, -1); i386_cleanup_dregs(); + if (open_process_used) + { + CHECK (CloseHandle (current_process_handle)); + open_process_used = 0; + } unpush_target (&win32_ops); generic_mourn_inferior (); } @@ -1960,11 +1969,6 @@ win32_kill_inferior (void) break; } - CHECK (CloseHandle (current_process_handle)); - - /* this may fail in an attached process so don't check. */ - if (current_thread && current_thread->h) - (void) CloseHandle (current_thread->h); target_mourn_inferior (); /* or just win32_mourn_inferior? */ }