fork() refactoring - Mailing list pgsql-patches

This patch moves all the common code that is usually invoked before
doing a fork() into a single function, fork_process(). It is not aware
of the EXEC_BACKEND machinery, so it should be used as fork() currently
is -- inside an #ifndef EXEC_BACKEND block, if appropriate.

I wasn't sure whether to put this under backend/port, postmaster.c, or
in a separate .c file. This patch places the code in a new file
postmaster/fork_process.c -- suggestions on whether this is the right
location would be welcome.

Barring any objections, I'll apply this to HEAD on Monday.

-Neil
Index: src/backend/port/beos/support.c
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql/src/backend/port/beos/support.c,v
retrieving revision 1.11
diff -c -r1.11 support.c
*** src/backend/port/beos/support.c    25 Oct 2004 03:23:02 -0000    1.11
--- src/backend/port/beos/support.c    5 Mar 2005 06:23:42 -0000
***************
*** 265,271 ****



! /* The behavior of fork is borken on beos regarding shared memory. In fact
  all shared memory areas are clones in copy on write mode in the new process.

  We need to do a remapping of these areas. Just afer the fork we performe the
--- 265,271 ----



! /* The behavior of fork is broken on beos regarding shared memory. In fact
  all shared memory areas are clones in copy on write mode in the new process.

  We need to do a remapping of these areas. Just afer the fork we performe the
Index: src/backend/postmaster/Makefile
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql/src/backend/postmaster/Makefile,v
retrieving revision 1.19
diff -c -r1.19 Makefile
*** src/backend/postmaster/Makefile    5 Aug 2004 23:32:10 -0000    1.19
--- src/backend/postmaster/Makefile    5 Mar 2005 06:37:28 -0000
***************
*** 12,18 ****
  top_builddir = ../../..
  include $(top_builddir)/src/Makefile.global

! OBJS = postmaster.o bgwriter.o pgstat.o pgarch.o syslogger.o

  all: SUBSYS.o

--- 12,18 ----
  top_builddir = ../../..
  include $(top_builddir)/src/Makefile.global

! OBJS = bgwriter.o fork_process.o pgarch.o pgstat.o postmaster.o syslogger.o

  all: SUBSYS.o

Index: src/backend/postmaster/fork_process.c
===================================================================
RCS file: src/backend/postmaster/fork_process.c
diff -N src/backend/postmaster/fork_process.c
*** /dev/null    1 Jan 1970 00:00:00 -0000
--- src/backend/postmaster/fork_process.c    5 Mar 2005 09:02:35 -0000
***************
*** 0 ****
--- 1,80 ----
+ /*
+  * fork_process.c
+  *   A simple wrapper on top of fork(). This does not handle the
+  *   EXEC_BACKEND case; it might be extended to do so, but it would be
+  *   considerably more complex.
+  *
+  * Copyright (c) 1996-2005, PostgreSQL Global Development Group
+  *
+  * IDENTIFICATION
+  *      $PostgreSQL$
+  */
+ #include "postgres.h"
+ #include "postmaster/fork_process.h"
+
+ #include <unistd.h>
+
+ /*
+  * Wrapper for fork(). Return values are the same as those for fork():
+  * -1 if the fork failed, 0 in the child process, and the PID of the
+  * child in the parent process.
+  */
+ pid_t
+ fork_process(void)
+ {
+     pid_t result;
+ #ifdef LINUX_PROFILE
+     struct itimerval prof_itimer;
+ #endif
+
+     /*
+      * Flush stdio channels just before fork, to avoid double-output
+      * problems. Ideally we'd use fflush(NULL) here, but there are still a
+      * few non-ANSI stdio libraries out there (like SunOS 4.1.x) that
+      * coredump if we do. Presently stdout and stderr are the only stdio
+      * output channels used by the postmaster, so fflush'ing them should
+      * be sufficient.
+      */
+     fflush(stdout);
+     fflush(stderr);
+
+ #ifdef LINUX_PROFILE
+     /*
+      * Linux's fork() resets the profiling timer in the child process. If
+      * we want to profile child processes then we need to save and restore
+      * the timer setting.  This is a waste of time if not profiling,
+      * however, so only do it if commanded by specific -DLINUX_PROFILE
+      * switch.
+      */
+     getitimer(ITIMER_PROF, &prof_itimer);
+ #endif
+
+ #ifdef __BEOS__
+     /* Specific beos actions before backend startup */
+     beos_before_backend_startup();
+ #endif
+
+     result = fork();
+     if (result == (pid_t) -1)
+     {
+         /* fork failed */
+ #ifdef __BEOS__
+         /* Specific beos backend startup actions */
+         beos_backend_startup_failed();
+ #endif
+     }
+     else if (result == 0)
+     {
+         /* fork succeeded, in child */
+ #ifdef LINUX_PROFILE
+         setitimer(ITIMER_PROF, &prof_itimer, NULL);
+ #endif
+
+ #ifdef __BEOS__
+         /* Specific beos backend startup actions */
+         beos_backend_startup();
+ #endif
+     }
+
+     return result;
+ }
Index: src/backend/postmaster/pgarch.c
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql/src/backend/postmaster/pgarch.c,v
retrieving revision 1.14
diff -c -r1.14 pgarch.c
*** src/backend/postmaster/pgarch.c    31 Dec 2004 22:00:40 -0000    1.14
--- src/backend/postmaster/pgarch.c    5 Mar 2005 08:29:38 -0000
***************
*** 34,39 ****
--- 34,40 ----
  #include "access/xlog_internal.h"
  #include "libpq/pqsignal.h"
  #include "miscadmin.h"
+ #include "postmaster/fork_process.h"
  #include "postmaster/pgarch.h"
  #include "postmaster/postmaster.h"
  #include "storage/fd.h"
***************
*** 141,165 ****
          return 0;
      last_pgarch_start_time = curtime;

-     fflush(stdout);
-     fflush(stderr);
-
- #ifdef __BEOS__
-     /* Specific beos actions before backend startup */
-     beos_before_backend_startup();
- #endif
-
  #ifdef EXEC_BACKEND
      switch ((pgArchPid = pgarch_forkexec()))
  #else
!     switch ((pgArchPid = fork()))
  #endif
      {
          case -1:
- #ifdef __BEOS__
-             /* Specific beos actions */
-             beos_backend_startup_failed();
- #endif
              ereport(LOG,
                      (errmsg("could not fork archiver: %m")));
              return 0;
--- 142,154 ----
          return 0;
      last_pgarch_start_time = curtime;

  #ifdef EXEC_BACKEND
      switch ((pgArchPid = pgarch_forkexec()))
  #else
!     switch ((pgArchPid = fork_process()))
  #endif
      {
          case -1:
              ereport(LOG,
                      (errmsg("could not fork archiver: %m")));
              return 0;
***************
*** 167,176 ****
  #ifndef EXEC_BACKEND
          case 0:
              /* in postmaster child ... */
- #ifdef __BEOS__
-             /* Specific beos actions after backend startup */
-             beos_backend_startup();
- #endif
              /* Close the postmaster's sockets */
              ClosePostmasterPorts(false);

--- 156,161 ----
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.445
diff -c -r1.445 postmaster.c
*** src/backend/postmaster/postmaster.c    22 Feb 2005 04:36:36 -0000    1.445
--- src/backend/postmaster/postmaster.c    5 Mar 2005 09:23:35 -0000
***************
*** 92,97 ****
--- 92,99 ----
  #include <DNSServiceDiscovery/DNSServiceDiscovery.h>
  #endif

+ #include "access/xlog.h"
+ #include "bootstrap/bootstrap.h"
  #include "catalog/pg_control.h"
  #include "catalog/pg_database.h"
  #include "commands/async.h"
***************
*** 103,125 ****
  #include "libpq/pqsignal.h"
  #include "miscadmin.h"
  #include "nodes/nodes.h"
! #include "postmaster/postmaster.h"
  #include "postmaster/pgarch.h"
  #include "postmaster/syslogger.h"
  #include "storage/fd.h"
  #include "storage/ipc.h"
  #include "storage/pg_shmem.h"
  #include "storage/pmsignal.h"
  #include "storage/proc.h"
- #include "storage/bufmgr.h"
- #include "access/xlog.h"
  #include "tcop/tcopprot.h"
  #include "utils/builtins.h"
  #include "utils/guc.h"
  #include "utils/memutils.h"
  #include "utils/ps_status.h"
- #include "bootstrap/bootstrap.h"
- #include "pgstat.h"

  #ifdef EXEC_BACKEND
  #include "storage/spin.h"
--- 105,126 ----
  #include "libpq/pqsignal.h"
  #include "miscadmin.h"
  #include "nodes/nodes.h"
! #include "pgstat.h"
! #include "postmaster/fork_process.h"
  #include "postmaster/pgarch.h"
+ #include "postmaster/postmaster.h"
  #include "postmaster/syslogger.h"
+ #include "storage/bufmgr.h"
  #include "storage/fd.h"
  #include "storage/ipc.h"
  #include "storage/pg_shmem.h"
  #include "storage/pmsignal.h"
  #include "storage/proc.h"
  #include "tcop/tcopprot.h"
  #include "utils/builtins.h"
  #include "utils/guc.h"
  #include "utils/memutils.h"
  #include "utils/ps_status.h"

  #ifdef EXEC_BACKEND
  #include "storage/spin.h"
***************
*** 1008,1023 ****
      int            i;
      pid_t        pid;

! #ifdef LINUX_PROFILE
!     struct itimerval prof_itimer;
! #endif
!
! #ifdef LINUX_PROFILE
!     /* see comments in BackendStartup */
!     getitimer(ITIMER_PROF, &prof_itimer);
! #endif
!
!     pid = fork();
      if (pid == (pid_t) -1)
      {
          write_stderr("%s: could not fork background process: %s\n",
--- 1009,1015 ----
      int            i;
      pid_t        pid;

!     pid = fork_process();
      if (pid == (pid_t) -1)
      {
          write_stderr("%s: could not fork background process: %s\n",
***************
*** 1030,1039 ****
          _exit(0);
      }

- #ifdef LINUX_PROFILE
-     setitimer(ITIMER_PROF, &prof_itimer, NULL);
- #endif
-
      MyProcPid = PostmasterPid = getpid();        /* reset PID vars to child */

  /* GH: If there's no setsid(), we hopefully don't need silent mode.
--- 1022,1027 ----
***************
*** 2382,2391 ****
      Backend    *bn;                /* for backend cleanup */
      pid_t        pid;

- #ifdef LINUX_PROFILE
-     struct itimerval prof_itimer;
- #endif
-
      /*
       * Compute the cancel key that will be assigned to this backend. The
       * backend will have its own copy in the forked-off process' value of
--- 2370,2375 ----
***************
*** 2409,2462 ****
      /* Pass down canAcceptConnections state (kluge for EXEC_BACKEND case) */
      port->canAcceptConnections = canAcceptConnections();

-     /*
-      * Flush stdio channels just before fork, to avoid double-output
-      * problems. Ideally we'd use fflush(NULL) here, but there are still a
-      * few non-ANSI stdio libraries out there (like SunOS 4.1.x) that
-      * coredump if we do. Presently stdout and stderr are the only stdio
-      * output channels used by the postmaster, so fflush'ing them should
-      * be sufficient.
-      */
-     fflush(stdout);
-     fflush(stderr);
-
  #ifdef EXEC_BACKEND
-
      pid = backend_forkexec(port);
-
  #else                            /* !EXEC_BACKEND */
!
! #ifdef LINUX_PROFILE
!
!     /*
!      * Linux's fork() resets the profiling timer in the child process. If
!      * we want to profile child processes then we need to save and restore
!      * the timer setting.  This is a waste of time if not profiling,
!      * however, so only do it if commanded by specific -DLINUX_PROFILE
!      * switch.
!      */
!     getitimer(ITIMER_PROF, &prof_itimer);
! #endif
!
! #ifdef __BEOS__
!     /* Specific beos actions before backend startup */
!     beos_before_backend_startup();
! #endif
!
!     pid = fork();
!
      if (pid == 0)                /* child */
      {
- #ifdef LINUX_PROFILE
-         setitimer(ITIMER_PROF, &prof_itimer, NULL);
- #endif
-
- #ifdef __BEOS__
-         /* Specific beos backend startup actions */
-         beos_backend_startup();
- #endif
          free(bn);
-
          proc_exit(BackendRun(port));
      }
  #endif   /* EXEC_BACKEND */
--- 2393,2405 ----
      /* Pass down canAcceptConnections state (kluge for EXEC_BACKEND case) */
      port->canAcceptConnections = canAcceptConnections();

  #ifdef EXEC_BACKEND
      pid = backend_forkexec(port);
  #else                            /* !EXEC_BACKEND */
!     pid = fork_process();
      if (pid == 0)                /* child */
      {
          free(bn);
          proc_exit(BackendRun(port));
      }
  #endif   /* EXEC_BACKEND */
***************
*** 2466,2475 ****
          /* in parent, fork failed */
          int            save_errno = errno;

- #ifdef __BEOS__
-         /* Specific beos backend startup actions */
-         beos_backend_startup_failed();
- #endif
          free(bn);
          errno = save_errno;
          ereport(LOG,
--- 2409,2414 ----
***************
*** 2944,2949 ****
--- 2883,2892 ----
      /* Insert temp file name after -fork argument */
      argv[2] = tmpfilename;

+     /* Flush stdout and stderr; see comment in fork_process */
+     fflush(stdout);
+     fflush(stderr);
+
      /* Fire off execv in child */
      if ((pid = fork()) == 0)
      {
***************
*** 3465,3474 ****
      int            ac = 0;
      char        xlbuf[32];

- #ifdef LINUX_PROFILE
-     struct itimerval prof_itimer;
- #endif
-
      /*
       * Set up command-line arguments for subprocess
       */
--- 3408,3413 ----
***************
*** 3488,3528 ****
      av[ac] = NULL;
      Assert(ac < lengthof(av));

-     /*
-      * Flush stdio channels (see comments in BackendStartup)
-      */
-     fflush(stdout);
-     fflush(stderr);
-
  #ifdef EXEC_BACKEND
-
      pid = postmaster_forkexec(ac, av);
-
  #else                            /* !EXEC_BACKEND */
!
! #ifdef LINUX_PROFILE
!     /* see comments in BackendStartup */
!     getitimer(ITIMER_PROF, &prof_itimer);
! #endif
!
! #ifdef __BEOS__
!     /* Specific beos actions before backend startup */
!     beos_before_backend_startup();
! #endif
!
!     pid = fork();

      if (pid == 0)                /* child */
      {
- #ifdef LINUX_PROFILE
-         setitimer(ITIMER_PROF, &prof_itimer, NULL);
- #endif
-
- #ifdef __BEOS__
-         /* Specific beos actions after backend startup */
-         beos_backend_startup();
- #endif
-
          IsUnderPostmaster = true;        /* we are a postmaster subprocess
                                           * now */

--- 3427,3439 ----
      av[ac] = NULL;
      Assert(ac < lengthof(av));

  #ifdef EXEC_BACKEND
      pid = postmaster_forkexec(ac, av);
  #else                            /* !EXEC_BACKEND */
!     pid = fork_process();

      if (pid == 0)                /* child */
      {
          IsUnderPostmaster = true;        /* we are a postmaster subprocess
                                           * now */

***************
*** 3546,3556 ****
      {
          /* in parent, fork failed */
          int            save_errno = errno;
-
- #ifdef __BEOS__
-         /* Specific beos actions before backend startup */
-         beos_backend_startup_failed();
- #endif
          errno = save_errno;
          switch (xlop)
          {
--- 3457,3462 ----
Index: src/backend/postmaster/syslogger.c
===================================================================
RCS file: /Users/neilc/local/cvs/pgsql/src/backend/postmaster/syslogger.c,v
retrieving revision 1.12
diff -c -r1.12 syslogger.c
*** src/backend/postmaster/syslogger.c    1 Jan 2005 20:44:16 -0000    1.12
--- src/backend/postmaster/syslogger.c    5 Mar 2005 06:48:51 -0000
***************
*** 33,38 ****
--- 33,39 ----

  #include "libpq/pqsignal.h"
  #include "miscadmin.h"
+ #include "postmaster/fork_process.h"
  #include "postmaster/postmaster.h"
  #include "postmaster/syslogger.h"
  #include "pgtime.h"
***************
*** 451,478 ****

      pfree(filename);

-     /*
-      * Now we can fork off the syslogger subprocess.
-      */
-     fflush(stdout);
-     fflush(stderr);
-
- #ifdef __BEOS__
-     /* Specific beos actions before backend startup */
-     beos_before_backend_startup();
- #endif
-
  #ifdef EXEC_BACKEND
      switch ((sysloggerPid = syslogger_forkexec()))
  #else
!     switch ((sysloggerPid = fork()))
  #endif
      {
          case -1:
- #ifdef __BEOS__
-             /* Specific beos actions */
-             beos_backend_startup_failed();
- #endif
              ereport(LOG,
                      (errmsg("could not fork system logger: %m")));
              return 0;
--- 452,464 ----

      pfree(filename);

  #ifdef EXEC_BACKEND
      switch ((sysloggerPid = syslogger_forkexec()))
  #else
!     switch ((sysloggerPid = fork_process()))
  #endif
      {
          case -1:
              ereport(LOG,
                      (errmsg("could not fork system logger: %m")));
              return 0;
***************
*** 480,489 ****
  #ifndef EXEC_BACKEND
          case 0:
              /* in postmaster child ... */
- #ifdef __BEOS__
-             /* Specific beos actions after backend startup */
-             beos_backend_startup();
- #endif
              /* Close the postmaster's sockets */
              ClosePostmasterPorts(true);

--- 466,471 ----
Index: src/include/postmaster/fork_process.h
===================================================================
RCS file: src/include/postmaster/fork_process.h
diff -N src/include/postmaster/fork_process.h
*** /dev/null    1 Jan 1970 00:00:00 -0000
--- src/include/postmaster/fork_process.h    5 Mar 2005 06:36:07 -0000
***************
*** 0 ****
--- 1,8 ----
+ #ifndef FORK_PROCESS_H
+ #define FORK_PROCESS_H
+
+ #include "postgres.h"
+
+ extern pid_t fork_process(void);
+
+ #endif /* ! FORK_PROCESS_H */