Re: 8.2.3: Server crashes on Windows using Eclipse/Junit

Dave Page wrote:
> Magnus Hagander wrote:
>> Right. You need to look at VM size in *process explorer*. VM size in
>> task manager has nothing to do with VM size, it's the private bytes :-S
>> And there is no way to see that info from task manager, I think. PE is
>> your friend.
>>
>>
>> Anyway. Other than a refresher on those, I'd be interested in two other
>> important parts:
>> * How many threads does it reach when you have 300 active backends?
>> * Is there a handle leak? meaning once your 300 backends have exited,
>> does the number of handles in the process drop down to the same value it
>> had before?
>
> Without patch:
>
> VM:             1,322,792K
> Idle threads:        6
> Peak threads:        306
> Handles at start:    576
> Handles at end:        576
>
> With patch:
>
> VM:             98,088K
> Idle threads:        3
> Peak threads:        7
> Handles at start:    576
> Handles at end:        585 (585 again after second run).

Ah, now we're talking. That's the kind of reduction I was looking for :-)

I think the difference in handles is because the threadpool keeps some
things around. As long as it stays at 585 and comes back down after a
second run, we're fine at that - there's no leak.

Attached is an updated version of the patch, currently being tested by
both me and Dave. If it passes our tests, I'll apply this so it gets
included for broader testing in beta2.


//Magnus
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.542
diff -c -r1.542 postmaster.c
*** src/backend/postmaster/postmaster.c    26 Sep 2007 22:36:30 -0000    1.542
--- src/backend/postmaster/postmaster.c    26 Oct 2007 20:09:35 -0000
***************
*** 331,344 ****
  #ifdef EXEC_BACKEND

  #ifdef WIN32
- static void win32_AddChild(pid_t pid, HANDLE handle);
- static void win32_RemoveChild(pid_t pid);
  static pid_t win32_waitpid(int *exitstatus);
! static DWORD WINAPI win32_sigchld_waiter(LPVOID param);

! static pid_t *win32_childPIDArray;
! static HANDLE *win32_childHNDArray;
! static unsigned long win32_numChildren = 0;

  HANDLE        PostmasterHandle;
  #endif
--- 331,347 ----
  #ifdef EXEC_BACKEND

  #ifdef WIN32
  static pid_t win32_waitpid(int *exitstatus);
! static void WINAPI pgwin32_deadchild_callback(PVOID lpParameter, BOOLEAN TimerOrWaitFired);

! static HANDLE win32ChildQueue;
!
! typedef struct
! {
!     HANDLE waitHandle;
!     HANDLE procHandle;
!     DWORD  procId;
! } win32_deadchild_waitinfo;

  HANDLE        PostmasterHandle;
  #endif
***************
*** 899,914 ****
  #ifdef WIN32

      /*
!      * Initialize the child pid/HANDLE arrays for signal handling.
       */
!     win32_childPIDArray = (pid_t *)
!         malloc(mul_size(NUM_BACKENDARRAY_ELEMS, sizeof(pid_t)));
!     win32_childHNDArray = (HANDLE *)
!         malloc(mul_size(NUM_BACKENDARRAY_ELEMS, sizeof(HANDLE)));
!     if (!win32_childPIDArray || !win32_childHNDArray)
          ereport(FATAL,
!                 (errcode(ERRCODE_OUT_OF_MEMORY),
!                  errmsg("out of memory")));

      /*
       * Set up a handle that child processes can use to check whether the
--- 902,913 ----
  #ifdef WIN32

      /*
!      * Initialize I/O completion port used to deliver list of dead children.
       */
!     win32ChildQueue = CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 1);
!     if (win32ChildQueue == NULL)
          ereport(FATAL,
!             (errmsg("could not create I/O completion port for child queue")));

      /*
       * Set up a handle that child processes can use to check whether the
***************
*** 2072,2083 ****
  #define LOOPHEADER()    (exitstatus = status.w_status)
  #else    /* WIN32 */
  #define LOOPTEST()        ((pid = win32_waitpid(&exitstatus)) > 0)
!     /*
!      * We need to do this here, and not in CleanupBackend, since this is
!      * to be called on all children when we are done with them. Could move
!      * to LogChildExit, but that seems like asking for future trouble...
!      */
! #define LOOPHEADER()    (win32_RemoveChild(pid))
  #endif   /* WIN32 */
  #endif   /* HAVE_WAITPID */

--- 2071,2077 ----
  #define LOOPHEADER()    (exitstatus = status.w_status)
  #else    /* WIN32 */
  #define LOOPTEST()        ((pid = win32_waitpid(&exitstatus)) > 0)
! #define LOOPHEADER()
  #endif   /* WIN32 */
  #endif   /* HAVE_WAITPID */

***************
*** 3332,3343 ****
      int            i;
      int            j;
      char        cmdLine[MAXPGPATH * 2];
-     HANDLE        childHandleCopy;
-     HANDLE        waiterThread;
      HANDLE        paramHandle;
      BackendParameters *param;
      SECURITY_ATTRIBUTES sa;
      char        paramHandleStr[32];

      /* Make sure caller set up argv properly */
      Assert(argc >= 3);
--- 3326,3336 ----
      int            i;
      int            j;
      char        cmdLine[MAXPGPATH * 2];
      HANDLE        paramHandle;
      BackendParameters *param;
      SECURITY_ATTRIBUTES sa;
      char        paramHandleStr[32];
+     win32_deadchild_waitinfo *childinfo;

      /* Make sure caller set up argv properly */
      Assert(argc >= 3);
***************
*** 3345,3359 ****
      Assert(strncmp(argv[1], "--fork", 6) == 0);
      Assert(argv[2] == NULL);

-     /* Verify that there is room in the child list */
-     if (win32_numChildren >= NUM_BACKENDARRAY_ELEMS)
-     {
-         elog(LOG, "no room for child entry in backend list");
-         /* Report same error as for a fork failure on Unix */
-         errno = EAGAIN;
-         return -1;
-     }
-
      /* Set up shared memory for parameter passing */
      ZeroMemory(&sa, sizeof(sa));
      sa.nLength = sizeof(sa);
--- 3338,3343 ----
***************
*** 3463,3496 ****
          return -1;
      }

!     if (!IsUnderPostmaster)
!     {
!         /* We are the Postmaster creating a child... */
!         win32_AddChild(pi.dwProcessId, pi.hProcess);
!     }

!     /* Set up the thread to handle the SIGCHLD for this process */
!     if (DuplicateHandle(GetCurrentProcess(),
!                         pi.hProcess,
!                         GetCurrentProcess(),
!                         &childHandleCopy,
!                         0,
!                         FALSE,
!                         DUPLICATE_SAME_ACCESS) == 0)
          ereport(FATAL,
!           (errmsg_internal("could not duplicate child handle: error code %d",
                             (int) GetLastError())));

!     waiterThread = CreateThread(NULL, 64 * 1024, win32_sigchld_waiter,
!                                 (LPVOID) childHandleCopy, 0, NULL);
!     if (!waiterThread)
!         ereport(FATAL,
!                 (errmsg_internal("could not create sigchld waiter thread: error code %d",
!                                  (int) GetLastError())));
!     CloseHandle(waiterThread);

-     if (IsUnderPostmaster)
-         CloseHandle(pi.hProcess);
      CloseHandle(pi.hThread);

      return pi.dwProcessId;
--- 3447,3479 ----
          return -1;
      }

!     /*
!      * Queue a waiter for to signal when this child dies. The wait will be handled automatically
!      * by an operating system thread pool.
!      *
!      * Note: use malloc instead of palloc, since it needs to be thread-safe
!      */
!     childinfo = malloc(sizeof(win32_deadchild_waitinfo));
!     if (!childinfo)
!         ereport(FATAL,
!           (errcode(ERRCODE_OUT_OF_MEMORY),
!            errmsg("out of memory")));

!     childinfo->procHandle = pi.hProcess;
!     childinfo->procId = pi.dwProcessId;
!
!     if (!RegisterWaitForSingleObject(&childinfo->waitHandle,
!                                      pi.hProcess,
!                                      pgwin32_deadchild_callback,
!                                      childinfo,
!                                      INFINITE,
!                                      WT_EXECUTEONLYONCE | WT_EXECUTEINWAITTHREAD))
          ereport(FATAL,
!           (errmsg_internal("could not register process for wait: error code %d",
                             (int) GetLastError())));

!     /* Don't close pi.hProcess here - the wait thread needs access to it */

      CloseHandle(pi.hThread);

      return pi.dwProcessId;
***************
*** 4500,4636 ****

  #ifdef WIN32

- /*
-  * Note: The following three functions must not be interrupted (eg. by
-  * signals).  As the Postgres Win32 signalling architecture (currently)
-  * requires polling, or APC checking functions which aren't used here, this
-  * is not an issue.
-  *
-  * We keep two separate arrays, instead of a single array of pid/HANDLE
-  * structs, to avoid having to re-create a handle array for
-  * WaitForMultipleObjects on each call to win32_waitpid.
-  */
-
- static void
- win32_AddChild(pid_t pid, HANDLE handle)
- {
-     Assert(win32_childPIDArray && win32_childHNDArray);
-     if (win32_numChildren < NUM_BACKENDARRAY_ELEMS)
-     {
-         win32_childPIDArray[win32_numChildren] = pid;
-         win32_childHNDArray[win32_numChildren] = handle;
-         ++win32_numChildren;
-     }
-     else
-         ereport(FATAL,
-                 (errmsg_internal("no room for child entry with pid %lu",
-                                  (unsigned long) pid)));
- }
-
- static void
- win32_RemoveChild(pid_t pid)
- {
-     int            i;
-
-     Assert(win32_childPIDArray && win32_childHNDArray);
-
-     for (i = 0; i < win32_numChildren; i++)
-     {
-         if (win32_childPIDArray[i] == pid)
-         {
-             CloseHandle(win32_childHNDArray[i]);
-
-             /* Swap last entry into the "removed" one */
-             --win32_numChildren;
-             win32_childPIDArray[i] = win32_childPIDArray[win32_numChildren];
-             win32_childHNDArray[i] = win32_childHNDArray[win32_numChildren];
-             return;
-         }
-     }
-
-     ereport(WARNING,
-             (errmsg_internal("could not find child entry with pid %lu",
-                              (unsigned long) pid)));
- }
-
  static pid_t
  win32_waitpid(int *exitstatus)
  {
      /*
!      * Note: Do NOT use WaitForMultipleObjectsEx, as we don't want to run
!      * queued APCs here.
       */
!     int            index;
!     DWORD        exitCode;
!     DWORD        ret;
!     unsigned long offset;
!
!     Assert(win32_childPIDArray && win32_childHNDArray);
!     elog(DEBUG3, "waiting on %lu children", win32_numChildren);
!
!     for (offset = 0; offset < win32_numChildren; offset += MAXIMUM_WAIT_OBJECTS)
      {
!         unsigned long num = Min(MAXIMUM_WAIT_OBJECTS, win32_numChildren - offset);
!
!         ret = WaitForMultipleObjects(num, &win32_childHNDArray[offset], FALSE, 0);
!         switch (ret)
!         {
!             case WAIT_FAILED:
!                 ereport(LOG,
!                         (errmsg_internal("failed to wait on %lu of %lu children: error code %d",
!                              num, win32_numChildren, (int) GetLastError())));
!                 return -1;
!
!             case WAIT_TIMEOUT:
!                 /* No children (in this chunk) have finished */
!                 break;
!
!             default:
!
!                 /*
!                  * Get the exit code, and return the PID of, the respective
!                  * process
!                  */
!                 index = offset + ret - WAIT_OBJECT_0;
!                 Assert(index >= 0 && index < win32_numChildren);
!                 if (!GetExitCodeProcess(win32_childHNDArray[index], &exitCode))
!                 {
!                     /*
!                      * If we get this far, this should never happen, but, then
!                      * again... No choice other than to assume a catastrophic
!                      * failure.
!                      */
!                     ereport(FATAL,
!                     (errmsg_internal("failed to get exit code for child %lu",
!                                (unsigned long) win32_childPIDArray[index])));
!                 }
!                 *exitstatus = (int) exitCode;
!                 return win32_childPIDArray[index];
!         }
      }

-     /* No children have finished */
      return -1;
  }

  /*
!  * Note! Code below executes on separate threads, one for
!  * each child process created
   */
! static DWORD WINAPI
! win32_sigchld_waiter(LPVOID param)
  {
!     HANDLE        procHandle = (HANDLE) param;

!     DWORD        r = WaitForSingleObject(procHandle, INFINITE);

!     if (r == WAIT_OBJECT_0)
!         pg_queue_signal(SIGCHLD);
!     else
!         write_stderr("could not wait on child process handle: error code %d\n",
!                      (int) GetLastError());
!     CloseHandle(procHandle);
!     return 0;
  }

  #endif   /* WIN32 */
--- 4483,4542 ----

  #ifdef WIN32

  static pid_t
  win32_waitpid(int *exitstatus)
  {
+     DWORD dwd;
+     ULONG_PTR key;
+     OVERLAPPED* ovl;
+
      /*
!      * Check if there are any dead children. If there are, return the pid of the first one that died.
       */
!     if (GetQueuedCompletionStatus(win32ChildQueue,&dwd,&key,&ovl,0))
      {
!         *exitstatus = (int)key;
!         return dwd;
      }

      return -1;
  }

  /*
!  * Note! Code below executes on a thread pool! All operations must
!  * be thread safe! Note that elog() and friends must *not* be used.
   */
! static void WINAPI
! pgwin32_deadchild_callback(PVOID lpParameter, BOOLEAN TimerOrWaitFired)
  {
!     win32_deadchild_waitinfo *childinfo = (win32_deadchild_waitinfo *)lpParameter;
!     DWORD        exitcode;

!     if (TimerOrWaitFired)
!         return;    /* timeout. Should never happen, since we use INFINITE as timeout value. */

!     /* Remove handle from wait - required even though it's set to wait only once */
!     UnregisterWaitEx(childinfo->waitHandle, NULL);
!
!     if (!GetExitCodeProcess(childinfo->procHandle, &exitcode))
!     {
!         /*
!          * Should never happen. Inform user and set a fixed exitcode.
!          */
!         write_stderr("could not read exitcode for process\n");
!         exitcode = 255;
!     }
!
!     if (!PostQueuedCompletionStatus(win32ChildQueue,childinfo->procId,(ULONG_PTR)exitcode,NULL))
!         write_stderr("could not post child completion status\n");
!
!     /* Handle is per-process, so we close it here instead of in the originating thread */
!     CloseHandle(childinfo->procHandle);
!
!     free(childinfo);
!
!     /* Queue SIGCHLD signal */
!     pg_queue_signal(SIGCHLD);
  }

  #endif   /* WIN32 */
Index: src/include/port/win32.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/port/win32.h,v
retrieving revision 1.76
diff -c -r1.76 win32.h
*** src/include/port/win32.h    25 Jul 2007 12:22:53 -0000    1.76
--- src/include/port/win32.h    26 Oct 2007 11:16:48 -0000
***************
*** 4,9 ****
--- 4,10 ----
  #define WIN32_ONLY_COMPILER
  #endif

+ #define _WIN32_WINNT 0x0500
  /*
   * Always build with SSPI support. Keep it as a #define in case
   * we want a switch to disable it sometime in the future.