Re: thread safety tests - Mailing list pgsql-hackers

Jan Wieck wrote:
> > I looked over the code and the only place getpwuid_r (through
> > pqGetpwuid) is used is in libpq to look up the default username based on
> > the euid for the connection to the backend.  Unfortunately, I can't find
> > any other way to do such a lookup in a thread-safe manner unless we do a
> > system() or lock/read /etc/passwd ourselves, both of which are ugly.
> >
> > I can't imagine how some OS's cannot give us a thread-safe way to do
> > this.
> >
> > When FreeBSD can't enable threads in 7.5, folks are going to be upset.
> > In 7.4 we allowed it by having our own C code lock/copy the passwd
> > structure, but someone pointed out that calls to getpwuid() in other
> > places in the client code don't have such locking, so it would not work,
> > so it was removed for 7.5.
>
> I disagree that all or nothing is a good strategy. What you have changed
> this to is to deny using PostgreSQL from multithreaded applications on
> platforms that have no getpwuid_r() altogether, if their platform
> happens to require any thread special compiler options for libpq to work
> in general.
>
> Take Slony as an example. It is multithreaded, and we aren't happy that
> we have to guard the pg_connect() call with a mutex against multiple
> concurrent calls. But since our connections are of long living nature
> this is no problem. And nowhere else in the entire code is any call to
> getpwuid() or anything else. So we have the situation under control. But
> I really don't want to tell people in the build instructions that they
> have to edit libpq's Makefile because PostgreSQL's ./configure script is
> too restrictive.

OK, I have added a configure option --enable-thread-safety-force that
compiles with thread safety even if the OS tests fail.  I have not
mentioned this in the SGML docs but mention the option in configure
--help and when you get a thread test failure with
--enable-thread-safety.

Patch attached and applied.

> And just to make your day, your tests for thread safety are incomplete.
> The reason why we use a mutex now on all platforms, thread safe or not,
> is because in the event you have a kerberos lib available (which is not
> thread safe), pg_connect() can crash wether you use kerberos or not. So
> I think when compiling for --enable-thread-safe we should disable
> kerberos in libpq, right?

We have libpq locking for kerberos in CVS.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: configure
===================================================================
RCS file: /cvsroot/pgsql-server/configure,v
retrieving revision 1.376
diff -c -c -r1.376 configure
*** configure    24 Jun 2004 18:55:17 -0000    1.376
--- configure    10 Jul 2004 01:08:16 -0000
***************
*** 846,851 ****
--- 846,852 ----
    --enable-depend         turn on automatic dependency tracking
    --enable-cassert        enable assertion checks (for debugging)
    --enable-thread-safety  make client libraries thread-safe
+   --enable-thread-safety-force  force thread-safety in spite of thread test failure
    --disable-largefile     omit support for large files

  Optional Packages:
***************
*** 2937,2947 ****

    case $enableval in
      yes)
!
! cat >>confdefs.h <<\_ACEOF
! #define ENABLE_THREAD_SAFETY 1
! _ACEOF
!
        ;;
      no)
        :
--- 2938,2944 ----

    case $enableval in
      yes)
!       :
        ;;
      no)
        :
***************
*** 2958,2963 ****
--- 2955,2994 ----

  fi;

+
+
+ # Check whether --enable-thread-safety-force or --disable-thread-safety-force was given.
+ if test "${enable_thread_safety_force+set}" = set; then
+   enableval="$enable_thread_safety_force"
+
+   case $enableval in
+     yes)
+       :
+       ;;
+     no)
+       :
+       ;;
+     *)
+       { { echo "$as_me:$LINENO: error: no argument expected for --enable-thread-safety-force option" >&5
+ echo "$as_me: error: no argument expected for --enable-thread-safety-force option" >&2;}
+    { (exit 1); exit 1; }; }
+       ;;
+   esac
+
+ else
+   enable_thread_safety_force=no
+
+ fi;
+
+ if test "$enable_thread_safety" = yes -o
+    test "$enable_thread_safety_force" = yes; then
+   enable_thread_safety="yes"    # for 'force'
+
+ cat >>confdefs.h <<\_ACEOF
+ #define ENABLE_THREAD_SAFETY 1
+ _ACEOF
+
+ fi
  echo "$as_me:$LINENO: result: $enable_thread_safety" >&5
  echo "${ECHO_T}$enable_thread_safety" >&6

***************
*** 17941,17947 ****
  # We have to run the thread test near the end so we have all our symbols
  # defined.  Cross compiling throws a warning.
  #
! if test "$enable_thread_safety" = yes; then
  echo "$as_me:$LINENO: checking thread safety of required library functions" >&5
  echo $ECHO_N "checking thread safety of required library functions... $ECHO_C" >&6

--- 17972,17991 ----
  # We have to run the thread test near the end so we have all our symbols
  # defined.  Cross compiling throws a warning.
  #
! if test "$enable_thread_safety_force" = yes; then
!   { echo "$as_me:$LINENO: WARNING:
! *** Skipping thread test program.  --enable-thread-safety-force was used.
! *** Run the program in src/tools/thread on the your machine and add
! proper locking function calls to your applications to guarantee thread
! safety.
! " >&5
! echo "$as_me: WARNING:
! *** Skipping thread test program.  --enable-thread-safety-force was used.
! *** Run the program in src/tools/thread on the your machine and add
! proper locking function calls to your applications to guarantee thread
! safety.
! " >&2;}
! elif test "$enable_thread_safety" = yes; then
  echo "$as_me:$LINENO: checking thread safety of required library functions" >&5
  echo $ECHO_N "checking thread safety of required library functions... $ECHO_C" >&6

***************
*** 17954,17964 ****
  echo "${ECHO_T}maybe" >&6
    { echo "$as_me:$LINENO: WARNING:
  *** Skipping thread test program because of cross-compile build.
! *** Run the program in src/tools/thread on the target matchine.
  " >&5
  echo "$as_me: WARNING:
  *** Skipping thread test program because of cross-compile build.
! *** Run the program in src/tools/thread on the target matchine.
  " >&2;}
  else
    cat >conftest.$ac_ext <<_ACEOF
--- 17998,18008 ----
  echo "${ECHO_T}maybe" >&6
    { echo "$as_me:$LINENO: WARNING:
  *** Skipping thread test program because of cross-compile build.
! *** Run the program in src/tools/thread on the target machine.
  " >&5
  echo "$as_me: WARNING:
  *** Skipping thread test program because of cross-compile build.
! *** Run the program in src/tools/thread on the target machine.
  " >&2;}
  else
    cat >conftest.$ac_ext <<_ACEOF
***************
*** 17988,17997 ****
  echo "${ECHO_T}no" >&6
    { { echo "$as_me:$LINENO: error:
  *** Thread test program failed.  Your platform is not thread-safe.
! *** Check the file 'config.log'for the exact reason." >&5
  echo "$as_me: error:
  *** Thread test program failed.  Your platform is not thread-safe.
! *** Check the file 'config.log'for the exact reason." >&2;}
     { (exit 1); exit 1; }; }
  fi
  rm -f core core.* *.core conftest$ac_exeext conftest.$ac_objext conftest.$ac_ext
--- 18032,18053 ----
  echo "${ECHO_T}no" >&6
    { { echo "$as_me:$LINENO: error:
  *** Thread test program failed.  Your platform is not thread-safe.
! *** Check the file 'config.log'for the exact reason.
! ***
! *** You can use the configure option --enable-thread-safety-force
! *** to force threads to be enabled.  However, you must then run
! *** the program in src/tools/thread and add locking function calls
! *** to your applications to guarantee thread safety.
! " >&5
  echo "$as_me: error:
  *** Thread test program failed.  Your platform is not thread-safe.
! *** Check the file 'config.log'for the exact reason.
! ***
! *** You can use the configure option --enable-thread-safety-force
! *** to force threads to be enabled.  However, you must then run
! *** the program in src/tools/thread and add locking function calls
! *** to your applications to guarantee thread safety.
! " >&2;}
     { (exit 1); exit 1; }; }
  fi
  rm -f core core.* *.core conftest$ac_exeext conftest.$ac_objext conftest.$ac_ext
Index: configure.in
===================================================================
RCS file: /cvsroot/pgsql-server/configure.in,v
retrieving revision 1.365
diff -c -c -r1.365 configure.in
*** configure.in    24 Jun 2004 18:55:18 -0000    1.365
--- configure.in    10 Jul 2004 01:08:17 -0000
***************
*** 358,366 ****
  # Enable thread-safe client libraries
  #
  AC_MSG_CHECKING([allow thread-safe client libraries])
! PGAC_ARG_BOOL(enable, thread-safety, no, [  --enable-thread-safety  make client libraries thread-safe],
!               [AC_DEFINE([ENABLE_THREAD_SAFETY], 1,
!                          [Define to 1 to build client libraries as thread-safe code. (--enable-thread-safety)])])
  AC_MSG_RESULT([$enable_thread_safety])
  AC_SUBST(enable_thread_safety)

--- 358,371 ----
  # Enable thread-safe client libraries
  #
  AC_MSG_CHECKING([allow thread-safe client libraries])
! PGAC_ARG_BOOL(enable, thread-safety, no, [  --enable-thread-safety  make client libraries thread-safe])
! PGAC_ARG_BOOL(enable, thread-safety-force, no, [  --enable-thread-safety-force  force thread-safety in spite of
threadtest failure]) 
! if test "$enable_thread_safety" = yes -o
!    test "$enable_thread_safety_force" = yes; then
!   enable_thread_safety="yes"    # for 'force'
!   AC_DEFINE([ENABLE_THREAD_SAFETY], 1,
!           [Define to 1 to build client libraries as thread-safe code. (--enable-thread-safety)])
! fi
  AC_MSG_RESULT([$enable_thread_safety])
  AC_SUBST(enable_thread_safety)

***************
*** 1184,1190 ****
  # We have to run the thread test near the end so we have all our symbols
  # defined.  Cross compiling throws a warning.
  #
! if test "$enable_thread_safety" = yes; then
  AC_MSG_CHECKING([thread safety of required library functions])

  _CFLAGS="$CFLAGS"
--- 1189,1202 ----
  # We have to run the thread test near the end so we have all our symbols
  # defined.  Cross compiling throws a warning.
  #
! if test "$enable_thread_safety_force" = yes; then
!   AC_MSG_WARN([
! *** Skipping thread test program.  --enable-thread-safety-force was used.
! *** Run the program in src/tools/thread on the your machine and add
! proper locking function calls to your applications to guarantee thread
! safety.
! ])
! elif test "$enable_thread_safety" = yes; then
  AC_MSG_CHECKING([thread safety of required library functions])

  _CFLAGS="$CFLAGS"
***************
*** 1196,1206 ****
    [AC_MSG_RESULT(no)
    AC_MSG_ERROR([
  *** Thread test program failed.  Your platform is not thread-safe.
! *** Check the file 'config.log'for the exact reason.])],
    [AC_MSG_RESULT(maybe)
    AC_MSG_WARN([
  *** Skipping thread test program because of cross-compile build.
! *** Run the program in src/tools/thread on the target matchine.
  ])])
  CFLAGS="$_CFLAGS"
  LIBS="$_LIBS"
--- 1208,1224 ----
    [AC_MSG_RESULT(no)
    AC_MSG_ERROR([
  *** Thread test program failed.  Your platform is not thread-safe.
! *** Check the file 'config.log'for the exact reason.
! ***
! *** You can use the configure option --enable-thread-safety-force
! *** to force threads to be enabled.  However, you must then run
! *** the program in src/tools/thread and add locking function calls
! *** to your applications to guarantee thread safety.
! ])],
    [AC_MSG_RESULT(maybe)
    AC_MSG_WARN([
  *** Skipping thread test program because of cross-compile build.
! *** Run the program in src/tools/thread on the target machine.
  ])])
  CFLAGS="$_CFLAGS"
  LIBS="$_LIBS"