Re: enable-thread-safety defaults? - Mailing list pgsql-hackers

Magnus Hagander wrote:
> 2009/11/24 Tom Lane <tgl@sss.pgh.pa.us>:
> > Magnus Hagander <magnus@hagander.net> writes:
> >> ISTM that it should be as simple as the attached patch. Seems to work
> >> for me :-) But I'm no autoconf guru, so maybe I missed something?
> >
> > This patch sort of begs the question "what about enable-thread-safety-force?"
> > That looks even more like a wart now than it did before.
>
> Agreed. But how about we try it piece-by-piece, which is we start with
> this to see if it actually hits any of our bf platforms?

Attached is a complete patch to enable threading of client libraries by
default --- I think it is time (threading was added to PG 7.4 in 2003).
I think we can guarantee that this will turn some build farm members
red.  How do we pass --disable-thread-safety to those hosts?

The patch also removes --enable-thread-safety-force, which was added in
2004 for a platform that didn't have a thread-safe getpwuid():

    http://archives.postgresql.org/pgsql-hackers/2004-07/msg00485.php

I think we can just tell people they have to upgrade their operating
systems if they want threading on those old platforms (or wait for
complaints).

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: configure
===================================================================
RCS file: /cvsroot/pgsql/configure,v
retrieving revision 1.659
diff -c -c -r1.659 configure
*** configure    30 Nov 2009 16:50:37 -0000    1.659
--- configure    30 Nov 2009 17:12:22 -0000
***************
*** 823,829 ****
  enable_depend
  enable_cassert
  enable_thread_safety
- enable_thread_safety_force
  with_tcl
  with_tclconfig
  with_perl
--- 823,828 ----
***************
*** 1497,1505 ****
    --enable-dtrace         build with DTrace support
    --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 despite thread test failure
    --disable-float4-byval  disable float4 passed by value
    --disable-float8-byval  disable float8 passed by value
    --disable-largefile     omit support for large files
--- 1496,1502 ----
    --enable-dtrace         build with DTrace support
    --enable-depend         turn on automatic dependency tracking
    --enable-cassert        enable assertion checks (for debugging)
!   --disable-thread-safety make client libraries thread-safe
    --disable-float4-byval  disable float4 passed by value
    --disable-float8-byval  disable float8 passed by value
    --disable-largefile     omit support for large files
***************
*** 4859,4892 ****
  #
  { $as_echo "$as_me:$LINENO: checking allow thread-safe client libraries" >&5
  $as_echo_n "checking allow thread-safe client libraries... " >&6; }
- if test "$PORTNAME" != "win32"; then
-
-
- # Check whether --enable-thread-safety was given.
- if test "${enable_thread_safety+set}" = set; then
-   enableval=$enable_thread_safety;
-   case $enableval in
-     yes)
-       :
-       ;;
-     no)
-       :
-       ;;
-     *)
-       { { $as_echo "$as_me:$LINENO: error: no argument expected for --enable-thread-safety option" >&5
- $as_echo "$as_me: error: no argument expected for --enable-thread-safety option" >&2;}
-    { (exit 1); exit 1; }; }
-       ;;
-   esac
-
- else
-   enable_thread_safety=no
-
- fi
-
-
- else
- # Win32 should always use threads


  # Check whether --enable-thread-safety was given.
--- 4856,4861 ----
***************
*** 4912,4953 ****
  fi


- fi
-
-
-
- # Check whether --enable-thread-safety-force was given.
- if test "${enable_thread_safety_force+set}" = set; then
-   enableval=$enable_thread_safety_force;
-   case $enableval in
-     yes)
-       :
-       ;;
-     no)
-       :
-       ;;
-     *)
-       { { $as_echo "$as_me:$LINENO: error: no argument expected for --enable-thread-safety-force option" >&5
- $as_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 \
-         "$enable_thread_safety_force" = yes; then
-   enable_thread_safety="yes"    # for 'force'
-
- cat >>confdefs.h <<\_ACEOF
- #define ENABLE_THREAD_SAFETY 1
- _ACEOF
-
- fi
  { $as_echo "$as_me:$LINENO: result: $enable_thread_safety" >&5
  $as_echo "$enable_thread_safety" >&6; }

--- 4881,4886 ----
***************
*** 21316,21325 ****
  if test "$PTHREAD_CC" != "$CC"; then
  { { $as_echo "$as_me:$LINENO: error:
  PostgreSQL does not support platforms that require a special compiler
! for thread safety." >&5
  $as_echo "$as_me: error:
  PostgreSQL does not support platforms that require a special compiler
! for thread safety." >&2;}
     { (exit 1); exit 1; }; }
  fi

--- 21249,21258 ----
  if test "$PTHREAD_CC" != "$CC"; then
  { { $as_echo "$as_me:$LINENO: error:
  PostgreSQL does not support platforms that require a special compiler
! for thread safety;  use --disable-thread-safety to disable threading." >&5
  $as_echo "$as_me: error:
  PostgreSQL does not support platforms that require a special compiler
! for thread safety;  use --disable-thread-safety to disable threading." >&2;}
     { (exit 1); exit 1; }; }
  fi

***************
*** 21465,21472 ****
  if test "x$ac_cv_header_pthread_h" = x""yes; then
    :
  else
!   { { $as_echo "$as_me:$LINENO: error: pthread.h not found, required for --enable-thread-safety" >&5
! $as_echo "$as_me: error: pthread.h not found, required for --enable-thread-safety" >&2;}
     { (exit 1); exit 1; }; }
  fi

--- 21398,21407 ----
  if test "x$ac_cv_header_pthread_h" = x""yes; then
    :
  else
!   { { $as_echo "$as_me:$LINENO: error:
! pthread.h not found;  use --disable-thread-safety to disable threading" >&5
! $as_echo "$as_me: error:
! pthread.h not found;  use --disable-thread-safety to disable threading" >&2;}
     { (exit 1); exit 1; }; }
  fi

***************
*** 26378,26387 ****
  if test "$pgac_cv_func_posix_signals" != yes -a "$enable_thread_safety" = yes; then
    { { $as_echo "$as_me:$LINENO: error:
  Thread-safety requires POSIX signals, which are not supported by this
! operating system." >&5
  $as_echo "$as_me: error:
  Thread-safety requires POSIX signals, which are not supported by this
! operating system." >&2;}
     { (exit 1); exit 1; }; }
  fi
  fi
--- 26313,26322 ----
  if test "$pgac_cv_func_posix_signals" != yes -a "$enable_thread_safety" = yes; then
    { { $as_echo "$as_me:$LINENO: error:
  Thread-safety requires POSIX signals, which are not supported by this
! operating system;  use --disable-thread-safety to disable threading." >&5
  $as_echo "$as_me: error:
  Thread-safety requires POSIX signals, which are not supported by this
! operating system;  use --disable-thread-safety to disable threading." >&2;}
     { (exit 1); exit 1; }; }
  fi
  fi
***************
*** 28231,28254 ****
  # 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
! if test "$PORTNAME" != "win32"
! then
!   { $as_echo "$as_me:$LINENO: WARNING:
! *** Skipping thread test program.  --enable-thread-safety-force was used.
! *** Run the program in src/test/thread on the your machine and add proper
! *** locking function calls to your applications to guarantee thread safety.
! " >&5
! $as_echo "$as_me: WARNING:
! *** Skipping thread test program.  --enable-thread-safety-force was used.
! *** Run the program in src/test/thread on the your machine and add proper
! *** locking function calls to your applications to guarantee thread safety.
! " >&2;}
! else
! { $as_echo "$as_me:$LINENO: WARNING: *** skipping thread test on Win32" >&5
! $as_echo "$as_me: WARNING: *** skipping thread test on Win32" >&2;}
! fi
! elif test "$enable_thread_safety" = yes; then
  if test "$PORTNAME" != "win32"
  then
  { $as_echo "$as_me:$LINENO: checking thread safety of required library functions" >&5
--- 28166,28172 ----
  # 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
  if test "$PORTNAME" != "win32"
  then
  { $as_echo "$as_me:$LINENO: checking thread safety of required library functions" >&5
***************
*** 28311,28331 ****
  { $as_echo "$as_me:$LINENO: result: no" >&5
  $as_echo "no" >&6; }
    { { $as_echo "$as_me:$LINENO: error: thread test program failed
! This 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.  But you must then run the program in
! src/test/thread and add locking function calls to your applications to
! guarantee thread safety." >&5
  $as_echo "$as_me: error: thread test program failed
! This 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.  But you must then run the program in
! src/test/thread and add locking function calls to your applications to
! guarantee thread safety." >&2;}
     { (exit 1); exit 1; }; }
  fi
  rm -rf conftest.dSYM
--- 28229,28241 ----
  { $as_echo "$as_me:$LINENO: result: no" >&5
  $as_echo "no" >&6; }
    { { $as_echo "$as_me:$LINENO: error: thread test program failed
! This platform is not thread-safe.  Check the file 'config.log' or compile
! and run src/test/thread/thread_test for the exact reason.
! Use --disable-thread-safety to disable threading." >&5
  $as_echo "$as_me: error: thread test program failed
! This platform is not thread-safe.  Check the file 'config.log' or compile
! and run src/test/thread/thread_test for the exact reason.
! Use --disable-thread-safety to disable threading." >&2;}
     { (exit 1); exit 1; }; }
  fi
  rm -rf conftest.dSYM
Index: configure.in
===================================================================
RCS file: /cvsroot/pgsql/configure.in,v
retrieving revision 1.613
diff -c -c -r1.613 configure.in
*** configure.in    30 Nov 2009 16:50:38 -0000    1.613
--- configure.in    30 Nov 2009 17:12:22 -0000
***************
*** 558,577 ****
  # Enable thread-safe client libraries
  #
  AC_MSG_CHECKING([allow thread-safe client libraries])
- if test "$PORTNAME" != "win32"; then
- PGAC_ARG_BOOL(enable, thread-safety, no, [make client libraries thread-safe])
- else
- # Win32 should always use threads
  PGAC_ARG_BOOL(enable, thread-safety, yes, [make client libraries thread-safe])
- fi
-
- PGAC_ARG_BOOL(enable, thread-safety-force, no, [force thread-safety despite thread test failure])
- if test "$enable_thread_safety" = yes -o \
-         "$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)

--- 558,564 ----
***************
*** 1404,1410 ****
  if test "$PTHREAD_CC" != "$CC"; then
  AC_MSG_ERROR([
  PostgreSQL does not support platforms that require a special compiler
! for thread safety.])
  fi

  # Check for *_r functions
--- 1391,1397 ----
  if test "$PTHREAD_CC" != "$CC"; then
  AC_MSG_ERROR([
  PostgreSQL does not support platforms that require a special compiler
! for thread safety;  use --disable-thread-safety to disable threading.])
  fi

  # Check for *_r functions
***************
*** 1414,1420 ****
  LIBS="$LIBS $PTHREAD_LIBS"

  if test "$PORTNAME" != "win32"; then
! AC_CHECK_HEADER(pthread.h, [], [AC_MSG_ERROR([pthread.h not found, required for --enable-thread-safety])])
  fi

  AC_CHECK_FUNCS([strerror_r getpwuid_r gethostbyname_r])
--- 1401,1408 ----
  LIBS="$LIBS $PTHREAD_LIBS"

  if test "$PORTNAME" != "win32"; then
! AC_CHECK_HEADER(pthread.h, [], [AC_MSG_ERROR([
! pthread.h not found;  use --disable-thread-safety to disable threading])])
  fi

  AC_CHECK_FUNCS([strerror_r getpwuid_r gethostbyname_r])
***************
*** 1660,1666 ****
  if test "$pgac_cv_func_posix_signals" != yes -a "$enable_thread_safety" = yes; then
    AC_MSG_ERROR([
  Thread-safety requires POSIX signals, which are not supported by this
! operating system.])
  fi
  fi

--- 1648,1654 ----
  if test "$pgac_cv_func_posix_signals" != yes -a "$enable_thread_safety" = yes; then
    AC_MSG_ERROR([
  Thread-safety requires POSIX signals, which are not supported by this
! operating system;  use --disable-thread-safety to disable threading.])
  fi
  fi

***************
*** 1749,1766 ****
  # 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
! if test "$PORTNAME" != "win32"
! then
!   AC_MSG_WARN([
! *** Skipping thread test program.  --enable-thread-safety-force was used.
! *** Run the program in src/test/thread on the your machine and add proper
! *** locking function calls to your applications to guarantee thread safety.
! ])
! else
! AC_MSG_WARN([*** skipping thread test on Win32])
! fi
! elif test "$enable_thread_safety" = yes; then
  if test "$PORTNAME" != "win32"
  then
  AC_MSG_CHECKING([thread safety of required library functions])
--- 1737,1743 ----
  # 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
  if test "$PORTNAME" != "win32"
  then
  AC_MSG_CHECKING([thread safety of required library functions])
***************
*** 1773,1785 ****
    [AC_MSG_RESULT(yes)],
    [AC_MSG_RESULT(no)
    AC_MSG_ERROR([thread test program failed
! This 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.  But you must then run the program in
! src/test/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.
--- 1750,1758 ----
    [AC_MSG_RESULT(yes)],
    [AC_MSG_RESULT(no)
    AC_MSG_ERROR([thread test program failed
! This platform is not thread-safe.  Check the file 'config.log' or compile
! and run src/test/thread/thread_test for the exact reason.
! Use --disable-thread-safety to disable threading.])],
    [AC_MSG_RESULT(maybe)
    AC_MSG_WARN([
  *** Skipping thread test program because of cross-compile build.
Index: doc/src/sgml/ecpg.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/ecpg.sgml,v
retrieving revision 1.92
diff -c -c -r1.92 ecpg.sgml
*** doc/src/sgml/ecpg.sgml    30 Nov 2009 14:47:37 -0000    1.92
--- doc/src/sgml/ecpg.sgml    30 Nov 2009 17:12:23 -0000
***************
*** 4709,4718 ****
    </para>

    <para>
!    The <application>ecpg</application> library is thread-safe if it is built
!    using the <option>--enable-thread-safety</> command-line option to
!    <filename>configure</filename>.  (You might need to use other threading
!    command-line options to compile your client code.)
    </para>
   </sect1>

--- 4709,4717 ----
    </para>

    <para>
!    The <application>ecpg</application> library is thread-safe by
!    default.  However, you might need to use some threading
!    command-line options to compile your client code.
    </para>
   </sect1>

Index: doc/src/sgml/installation.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/installation.sgml,v
retrieving revision 1.327
diff -c -c -r1.327 installation.sgml
*** doc/src/sgml/installation.sgml    12 Aug 2009 16:32:35 -0000    1.327
--- doc/src/sgml/installation.sgml    30 Nov 2009 17:12:23 -0000
***************
*** 1164,1177 ****
        </varlistentry>

        <varlistentry>
!        <term><option>--enable-thread-safety</option></term>
         <listitem>
          <para>
!          Make the client libraries thread-safe.  This allows
           concurrent threads in <application>libpq</application> and
!          <application>ECPG</application> programs to safely control
!          their private connection handles.  This option requires adequate
!          threading support in your operating system.
          </para>
         </listitem>
        </varlistentry>
--- 1164,1176 ----
        </varlistentry>

        <varlistentry>
!        <term><option>--disable-thread-safety</option></term>
         <listitem>
          <para>
!          Disable the thread-safety of client libraries.  This prevents
           concurrent threads in <application>libpq</application> and
!          <application>ECPG</application> programs from safely controlling
!          their private connection handles.
          </para>
         </listitem>
        </varlistentry>
***************
*** 2835,2846 ****
     </sect3>

     <sect3>
!     <title><option>--enable-thread-safety</option> and UnixWare</title>

      <para>
!      If you use the <command>configure</command>
!      option <option>--enable-thread-safety</option>,
!      you <emphasis>must</emphasis> use <option>-Kpthread</option>
       on <emphasis>all</emphasis> libpq-using programs.  libpq
       uses <function>pthread_*</function> calls, which are only
       available with the
--- 2834,2843 ----
     </sect3>

     <sect3>
!     <title>Threading on UnixWare</title>

      <para>
!      For threading, you<emphasis>must</emphasis> use <option>-Kpthread</option>
       on <emphasis>all</emphasis> libpq-using programs.  libpq
       uses <function>pthread_*</function> calls, which are only
       available with the
Index: doc/src/sgml/libpq.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.290
diff -c -c -r1.290 libpq.sgml
*** doc/src/sgml/libpq.sgml    28 Nov 2009 23:38:06 -0000    1.290
--- doc/src/sgml/libpq.sgml    30 Nov 2009 17:12:23 -0000
***************
*** 6577,6587 ****
    </indexterm>

    <para>
!    <application>libpq</application> is reentrant and thread-safe if the
!    <filename>configure</filename> command-line option
!    <literal>--enable-thread-safety</> was used when the
!    <productname>PostgreSQL</productname> distribution was built.  In
!    addition, you might need to use additional compiler command-line
     options when you compile your application code.  Refer to your
     system's documentation for information about how to build
     thread-enabled applications, or look in
--- 6577,6584 ----
    </indexterm>

    <para>
!    <application>libpq</application> is reentrant and thread-safe by default.
!    You might need to use special compiler command-line
     options when you compile your application code.  Refer to your
     system's documentation for information about how to build
     thread-enabled applications, or look in
Index: src/test/thread/README
===================================================================
RCS file: /cvsroot/pgsql/src/test/thread/README,v
retrieving revision 1.2
diff -c -c -r1.2 README
*** src/test/thread/README    21 Mar 2008 13:23:29 -0000    1.2
--- src/test/thread/README    30 Nov 2009 17:12:23 -0000
***************
*** 3,17 ****
  Threading
  =========

! This program should be run by developers wishing to enable threading on
! new platforms.

! Run thread_test program to determine if your native libc functions are
! thread-safe, or if we should use *_r functions or thread locking.

! To use this program, you must:

!     o run "configure --enable-thread-safety"
      o compile the main source tree
      o compile and run this program

--- 3,18 ----
  Threading
  =========

! This program is run by configure to determine if threading is
! properly supported on the platform.

! You can run the program manually to see details, which shows if your
! native libc functions are thread-safe, or if we use *_r functions or
! thread locking.

! To use this program manually, you must:

!     o run "configure"
      o compile the main source tree
      o compile and run this program