[RFA] gdb/win32-nat.c: do not call CloseHandle on process and thread handles

[RFA] gdb/win32-nat.c: do not call CloseHandle on process and thread handles

[Pierre Muller]

  Following the discussion 
http://sourceware.org/ml/gdb/2007-10/msg00131.html
and
http://sourceware.org/ml/gdb-patches/2007-10/msg00502.html

  I resubmit a part of my original patch that only contains the
removal of all calls to the win32 API CloseHandle
with thread or process handles.

  This patch allows to debug gdb with itself on mingw32
port. 

  The patch was tested for cygwin target, by running
the testsuite with and without this patch.

  The testsuite shows no changes.

Pierre Muller

PS: I am unsure about the proper way to decribe the change 
in the ChangeLog...


ChangeLog entry:

2007-11-22  Pierre Muller  <muller@ics.u-strasbg.fr>

	*win32-nat.c: Remove calls to CloseHandle for thread and process
handles.
	(win32_init_thread_list): Ditto.
	(win32_delete_thread): Ditto.
	(get_win32_debug_event): Ditto.
	(win32_create_inferior): Ditto.
	(win32_kill_inferior): Ditto.

Index: gdb/win32-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/win32-nat.c,v
retrieving revision 1.139
diff -u -p -r1.139 win32-nat.c
--- gdb/win32-nat.c     16 Nov 2007 04:53:46 -0000      1.139
+++ gdb/win32-nat.c     22 Nov 2007 10:07:30 -0000
@@ -316,7 +316,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;
@@ -341,7 +340,6 @@ win32_delete_thread (DWORD id)
     {
       thread_info *here = th->next;
       th->next = here->next;
-      CloseHandle (here->h);
       xfree (here);
     }
 }
@@ -1323,7 +1321,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;
        }

@@ -1346,7 +1343,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;

@@ -1876,9 +1872,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
@@ -1950,11 +1943,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? */
 }

Re: [RFA] gdb/win32-nat.c: do not call CloseHandle on process and thread handles

[Pedro Alves]

Hi Pierre,

Pierre Muller wrote:
>   Following the discussion
> http://sourceware.org/ml/gdb/2007-10/msg00131.html
> and
> http://sourceware.org/ml/gdb-patches/2007-10/msg00502.html
>
>   I resubmit a part of my original patch that only contains the
> removal of all calls to the win32 API CloseHandle
> with thread or process handles.
>

You'll still have to somehow close this process handle (current_process_handle):

/* Called in pathological case where Windows fails to send a
   CREATE_PROCESS_DEBUG_EVENT after an attach.  */
static DWORD
fake_create_process (void)
{
  current_process_handle = OpenProcess (PROCESS_ALL_ACCESS, FALSE,
                                        current_event.dwProcessId);
  main_thread_id = current_event.dwThreadId;
  current_thread = win32_add_thread (main_thread_id,
                                     current_event.u.CreateThread.hThread);
  return main_thread_id;
}

-- 
Pedro Alves

RE: [RFA] gdb/win32-nat.c: do not call CloseHandle on process and thread handles

[Pierre Muller]

> You'll still have to somehow close this process handle
(current_process_handle):
>
> /* Called in pathological case where Windows fails to send a
>    CREATE_PROCESS_DEBUG_EVENT after an attach.  */ 
> static DWORD fake_create_process (void) {
>   current_process_handle = OpenProcess (PROCESS_ALL_ACCESS, FALSE,
>                                         current_event.dwProcessId);
>   main_thread_id = current_event.dwThreadId;
>   current_thread = win32_add_thread (main_thread_id,
>
current_event.u.CreateThread.hThread);
>   return main_thread_id;
> }

I don't think that anything in the testsuite checks
this case.


  The only code location that calls this function has also some comment:

    case CREATE_THREAD_DEBUG_EVENT:
      DEBUG_EVENTS (("gdb: kernel event for pid=%d tid=%x code=%s)\n",
                     (unsigned) current_event.dwProcessId,
                     (unsigned) current_event.dwThreadId,
                     "CREATE_THREAD_DEBUG_EVENT"));
      if (saw_create != 1)
        {
          if (!saw_create && attach_flag)
            {
              /* Kludge around a Windows bug where first event is a create
                 thread event.  Caused when attached process does not have
                 a main thread. */
              retval = ourstatus->value.related_pid = fake_create_process
();
              saw_create++;
            }

  But what does the wording 'does not have a main thread' mean?
Is there a way to create such a process in order to try to find out
how we need to solve this issue?
  I tried to modify your suspend source to exit the main thread before
the secondary thread exists, but it is unclear to me if
the main thread can really be terminated by a call to ExitThread.
See modified source below.
  I do get a EXIT_THREAD_DEBUG_EVENT, (as can be seen if you use 'set
debugevents 1')
but get_win32_debug_event ignores this event if the id is equal to
main_thread_id,
so that the thread seems still valid for gdb.

  On the other hand, I tried to get the GetExitCodeThread,
but this call always returns STILL_ACTIVE...
Thus I could not create a win32 executable that has no 
main thread anymore...
  Does anyone have such a code ready?

Concerning attached processes that send a CREATE_THREAD_DEBUG_EVENT first,
is the CREATE_PROCESS_DEBUG_EVENT event still sent to debugger,
but after the CREATE_THREAD_DEBUG_EVENT?

  In that case, it might be better to close the opened handle
when the CREATE_PROCESS_DEBUG_EVENT arrives and switch 
current_process_handle to the handle value given at 
that time.

Pierre Muller

Modified code to try to get a program that has no main thread anymore:

#include <windows.h>

HANDLE started;
HANDLE stop;
HANDLE mainh;

DWORD WINAPI
thread_start (void *arg)
{
        int i;
        SetEvent (started);
        WaitForSingleObject (stop, INFINITE);
        for (i=1;i<=1000;i++)
          {
            DWORD ex;
            Sleep(100);
            if (!GetExitCodeThread (mainh, &ex))
              printf("GetExitCodeThread failed %d\n", GetLastError ());
            else
              if (ex != STILL_ACTIVE)
                printf ("main thread exited with value %d\n", ex);
            printf("%d\n",i);
          }
        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);
        mainh = GetCurrentThread ();
        ExitThread (6);
        return 0;
}

Re: [RFA] gdb/win32-nat.c: do not call CloseHandle on process and thread handles

[Pedro Alves]

Pierre Muller wrote:
> > You'll still have to somehow close this process handle
> (current_process_handle):
> >
> > /* Called in pathological case where Windows fails to send a
> >    CREATE_PROCESS_DEBUG_EVENT after an attach.  */
> > static DWORD fake_create_process (void) {
> >   current_process_handle = OpenProcess (PROCESS_ALL_ACCESS, FALSE,
> >                                         current_event.dwProcessId);
> >   main_thread_id = current_event.dwThreadId;
> >   current_thread = win32_add_thread (main_thread_id,
> >
> current_event.u.CreateThread.hThread);
> >   return main_thread_id;
> > }
>
> I don't think that anything in the testsuite checks
> this case.
>
>
>   The only code location that calls this function has also some comment:
>
>     case CREATE_THREAD_DEBUG_EVENT:
>       DEBUG_EVENTS (("gdb: kernel event for pid=%d tid=%x code=%s)\n",
>                      (unsigned) current_event.dwProcessId,
>                      (unsigned) current_event.dwThreadId,
>                      "CREATE_THREAD_DEBUG_EVENT"));
>       if (saw_create != 1)
>         {
>           if (!saw_create && attach_flag)
>             {
>               /* Kludge around a Windows bug where first event is a create
>                  thread event.  Caused when attached process does not have
>                  a main thread. */
>               retval = ourstatus->value.related_pid = fake_create_process
> ();
>               saw_create++;
>             }
>
>   But what does the wording 'does not have a main thread' mean?
> Is there a way to create such a process in order to try to find out
> how we need to solve this issue?

Try attaching to the example posted at threads/1048.

> Concerning attached processes that send a CREATE_THREAD_DEBUG_EVENT first,
> is the CREATE_PROCESS_DEBUG_EVENT event still sent to debugger,
> but after the CREATE_THREAD_DEBUG_EVENT?
>

Don't know, you'll have to check, but I doubt it.
It just might be easier to always open a handle to the process (OpenProcess),
and not touch the one coming on the event.  Then you would always close
the process handle, because you know you're the one who opened it.  OTTOMH,
gdbserver does something similar, but leaks.
This uses an extra handle on the normal case, but I don't think I'd care, as
long as there are no leaks. Otherwise, you'll just have to keep a flag
somewhere.

-- 
Pedro Alves

Re: [RFA] gdb/win32-nat.c: do not call CloseHandle on process and thread handles

[Christopher Faylor]

On Thu, Nov 22, 2007 at 02:37:26PM +0000, Pedro Alves wrote:
>Don't know, you'll have to check, but I doubt it.  It just might be
>easier to always open a handle to the process (OpenProcess), and not
>touch the one coming on the event.  Then you would always close the
>process handle, because you know you're the one who opened it.  OTTOMH,
>gdbserver does something similar, but leaks.  This uses an extra handle
>on the normal case, but I don't think I'd care, as long as there are no
>leaks.  Otherwise, you'll just have to keep a flag somewhere.

Are you sure that it's always possible for the debugger to open a handle
to the process?  It is a given that there will always be a handle
available via the debugging interface but I don't know that it is a
given that a nonprivileged process would necessarily be able to open a
handle to a privileged process.  I think I'd prefer a flag.

And, it should be easy to create a program which has no main thread.
Just write a program which has two threads, exit the main thread and
attach to the program.

cgf

[RFA] gdb/win32-nat.c: do not call CloseHandle on process and thread handles

[Pierre Muller]

  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  <muller@ics.u-strasbg.fr>

	*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.


Index: gdb/win32-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/win32-nat.c,v
retrieving revision 1.139
diff -u -p -r1.139 win32-nat.c
--- gdb/win32-nat.c     16 Nov 2007 04:53:46 -0000      1.139
+++ gdb/win32-nat.c     23 Nov 2007 13:30:39 -0000
@@ -140,6 +140,7 @@ static DWORD main_thread_id;                /* Thread
 static int exception_count = 0;
 static int event_count = 0;
 static int saw_create;
+static int fake_create_used = 0;

 /* User options. */
 static int new_console = 0;
@@ -316,7 +317,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;
@@ -341,7 +341,6 @@ win32_delete_thread (DWORD id)
     {
       thread_info *here = th->next;
       th->next = here->next;
-      CloseHandle (here->h);
       xfree (here);
     }
 }
@@ -1160,6 +1159,17 @@ fake_create_process (void)
 {
   current_process_handle = OpenProcess (PROCESS_ALL_ACCESS, FALSE,
                                        current_event.dwProcessId);
+  if (current_process_handle != NULL)
+    {
+      fake_create_used = 1;
+    }
+  else
+    {
+      printf_unfiltered ("OpenProcess call failed, GetLastError = %lud\n",
+       GetLastError ());
+      /*  We can not debug anything in that case.  */
+      return 0;
+    }
   main_thread_id = current_event.dwThreadId;
   current_thread = win32_add_thread (main_thread_id,
                                     current_event.u.CreateThread.hThread);
@@ -1289,7 +1299,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;
        }
@@ -1323,7 +1334,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;
        }

@@ -1346,7 +1356,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;

@@ -1478,6 +1487,7 @@ do_initial_win32_stuff (DWORD pid)
   last_sig = TARGET_SIGNAL_0;
   event_count = 0;
   exception_count = 0;
+  fake_create_used = 0;
   debug_registers_changed = 0;
   debug_registers_used = 0;
   for (i = 0; i < sizeof (dr) / sizeof (dr[0]); i++)
@@ -1876,9 +1886,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
@@ -1894,6 +1901,11 @@ win32_mourn_inferior (void)
 {
   (void) win32_continue (DBG_CONTINUE, -1);
   i386_cleanup_dregs();
+  if (fake_create_used)
+    {
+      CHECK (CloseHandle (current_process_handle));
+      fake_create_used = 0;
+    }
   unpush_target (&win32_ops);
   generic_mourn_inferior ();
 }
@@ -1950,11 +1962,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? */
 }

Re: [RFA] gdb/win32-nat.c: do not call CloseHandle on process and thread handles

[Christopher Faylor]

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  <muller@ics.u-strasbg.fr>
>
>	*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.

Also, rather than using printf_unfiltered, shouldn't you be using error,
like the rest of the code?

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

[RFA 2] gdb/win32-nat.c: do not call CloseHandle on process and thread handles

[Pierre Muller]

> -----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  <muller@ics.u-strasbg.fr>
> >
> >	*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  <muller@ics.u-strasbg.fr>

	*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? */
 }

Re: [RFA 2] gdb/win32-nat.c: do not call CloseHandle on process and thread handles

[Christopher Faylor]

On Mon, Nov 26, 2007 at 10:21:42AM +0100, Pierre Muller wrote:
>2007-11-26  Pierre Muller  <muller@ics.u-strasbg.fr>
>
>	*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.

Approved and applied.

Sorry for the delay.  My cygwin build script for gdb is showing its age
and need some kicking with steel-toed shoes.

cgf

Re: [RFA] gdb/win32-nat.c: do not call CloseHandle on process and thread handles

[Pedro Alves]

On Nov 23, 2007 1:17 AM, Christopher Faylor wrote:
> On Thu, Nov 22, 2007 at 02:37:26PM +0000, Pedro Alves wrote:
> >Don't know, you'll have to check, but I doubt it.  It just might be
> >easier to always open a handle to the process (OpenProcess), and not
> >touch the one coming on the event.  Then you would always close the
> >process handle, because you know you're the one who opened it.  OTTOMH,
> >gdbserver does something similar, but leaks.  This uses an extra handle
> >on the normal case, but I don't think I'd care, as long as there are no
> >leaks.  Otherwise, you'll just have to keep a flag somewhere.
>
> Are you sure that it's always possible for the debugger to open a handle
> to the process?  It is a given that there will always be a handle
> available via the debugging interface but I don't know that it is a
> given that a nonprivileged process would necessarily be able to open a
> handle to a privileged process.  I think I'd prefer a flag.
>

I'd be surprised if you could be able to debug/run/'attach to' a
process, but not be
able to get a handle to it.  Anyhow, FWIW, I'm OK with a flag as well.

-- 
Pedro Alves