Diagnose incompatible OpenLDAP versions during build and test.
authorNoah Misch <noah@leadboat.com>
Tue, 22 Jul 2014 15:01:03 +0000 (11:01 -0400)
committerNoah Misch <noah@leadboat.com>
Tue, 22 Jul 2014 15:01:41 +0000 (11:01 -0400)
With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, PostgreSQL
backends can crash at exit.  Raise a warning during "configure" based on
the compile-time OpenLDAP version number, and test the crash scenario in
the dblink test suite.  Back-patch to 9.0 (all supported versions).

configure
configure.in
contrib/dblink/Makefile
contrib/dblink/expected/.gitignore [new file with mode: 0644]
contrib/dblink/expected/dblink.out
contrib/dblink/input/paths.source [new file with mode: 0644]
contrib/dblink/output/paths.source [new file with mode: 0644]
contrib/dblink/pg_service.conf [new file with mode: 0644]
contrib/dblink/sql/.gitignore [new file with mode: 0644]
contrib/dblink/sql/dblink.sql
src/test/regress/regress.c

index bedd09357dde232b58e359de0279842c539eb303..5136270bb294c43f7b29636cf615a637c7abd021 100755 (executable)
--- a/configure
+++ b/configure
 
 fi
 
+# PGAC_LDAP_SAFE
+# --------------
+# PostgreSQL sometimes loads libldap_r and plain libldap into the same
+# process.  Check for OpenLDAP versions known not to tolerate doing so; assume
+# non-OpenLDAP implementations are safe.  The dblink test suite exercises the
+# hazardous interaction directly.
+
+
+
+
+
 if test "$with_ldap" = yes ; then
   if test "$PORTNAME" != "win32"; then
 
 
 done
 
+     { $as_echo "$as_me:$LINENO: checking for compatible LDAP implementation" >&5
+$as_echo_n "checking for compatible LDAP implementation... " >&6; }
+if test "${pgac_cv_ldap_safe+set}" = set; then
+  $as_echo_n "(cached) " >&6
+else
+  cat >conftest.$ac_ext <<_ACEOF
+/* confdefs.h.  */
+_ACEOF
+cat confdefs.h >>conftest.$ac_ext
+cat >>conftest.$ac_ext <<_ACEOF
+/* end confdefs.h.  */
+#include <ldap.h>
+#if !defined(LDAP_VENDOR_VERSION) || \
+     (defined(LDAP_API_FEATURE_X_OPENLDAP) && \
+      LDAP_VENDOR_VERSION >= 20424 && LDAP_VENDOR_VERSION <= 20431)
+choke me
+#endif
+int
+main ()
+{
+
+  ;
+  return 0;
+}
+_ACEOF
+rm -f conftest.$ac_objext
+if { (ac_try="$ac_compile"
+case "(($ac_try" in
+  *\"* | *\`* | *\\*) ac_try_echo=\$ac_try;;
+  *) ac_try_echo=$ac_try;;
+esac
+eval ac_try_echo="\"\$as_me:$LINENO: $ac_try_echo\""
+$as_echo "$ac_try_echo") >&5
+  (eval "$ac_compile") 2>conftest.er1
+  ac_status=$?
+  grep -v '^ *+' conftest.er1 >conftest.err
+  rm -f conftest.er1
+  cat conftest.err >&5
+  $as_echo "$as_me:$LINENO: \$? = $ac_status" >&5
+  (exit $ac_status); } && {
+    test -z "$ac_c_werror_flag" ||
+    test ! -s conftest.err
+       } && test -s conftest.$ac_objext; then
+  pgac_cv_ldap_safe=yes
+else
+  $as_echo "$as_me: failed program was:" >&5
+sed 's/^/| /' conftest.$ac_ext >&5
+
+   pgac_cv_ldap_safe=no
+fi
+
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+fi
+{ $as_echo "$as_me:$LINENO: result: $pgac_cv_ldap_safe" >&5
+$as_echo "$pgac_cv_ldap_safe" >&6; }
+
+if test "$pgac_cv_ldap_safe" != yes; then
+  { $as_echo "$as_me:$LINENO: WARNING:
+*** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend
+*** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and
+*** also uses LDAP will crash on exit." >&5
+$as_echo "$as_me: WARNING:
+*** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend
+*** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and
+*** also uses LDAP will crash on exit." >&2;}
+fi
   else
 
 for ac_header in winldap.h
index 04d1092e0c33df00456a8e889500f74558a32c42..e946f6503d22ed77c530185393571285e49fedad 100644 (file)
@@ -1080,10 +1080,39 @@ if test "$with_libxslt" = yes ; then
   AC_CHECK_HEADER(libxslt/xslt.h, [], [AC_MSG_ERROR([header file <libxslt/xslt.h> is required for XSLT support])])
 fi
 
+# PGAC_LDAP_SAFE
+# --------------
+# PostgreSQL sometimes loads libldap_r and plain libldap into the same
+# process.  Check for OpenLDAP versions known not to tolerate doing so; assume
+# non-OpenLDAP implementations are safe.  The dblink test suite exercises the
+# hazardous interaction directly.
+
+AC_DEFUN([PGAC_LDAP_SAFE],
+[AC_CACHE_CHECK([for compatible LDAP implementation], [pgac_cv_ldap_safe],
+[AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
+[#include <ldap.h>
+#if !defined(LDAP_VENDOR_VERSION) || \
+     (defined(LDAP_API_FEATURE_X_OPENLDAP) && \
+      LDAP_VENDOR_VERSION >= 20424 && LDAP_VENDOR_VERSION <= 20431)
+choke me
+#endif], [])],
+[pgac_cv_ldap_safe=yes],
+[pgac_cv_ldap_safe=no])])
+
+if test "$pgac_cv_ldap_safe" != yes; then
+  AC_MSG_WARN([
+*** With OpenLDAP versions 2.4.24 through 2.4.31, inclusive, each backend
+*** process that loads libpq (via WAL receiver, dblink, or postgres_fdw) and
+*** also uses LDAP will crash on exit.])
+fi])
+
+
+
 if test "$with_ldap" = yes ; then
   if test "$PORTNAME" != "win32"; then
      AC_CHECK_HEADERS(ldap.h, [],
                       [AC_MSG_ERROR([header file <ldap.h> is required for LDAP])])
+     PGAC_LDAP_SAFE
   else
      AC_CHECK_HEADERS(winldap.h, [],
                       [AC_MSG_ERROR([header file <winldap.h> is required for LDAP])],
index 32314a0abb48e228507080ee989af922c1f7150b..09032f89702a4aee07fa819d2fb15f83de72ab34 100644 (file)
@@ -9,7 +9,9 @@ SHLIB_PREREQS = submake-libpq
 EXTENSION = dblink
 DATA = dblink--1.1.sql dblink--1.0--1.1.sql dblink--unpackaged--1.0.sql
 
-REGRESS = dblink
+REGRESS = paths dblink
+REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
+EXTRA_CLEAN = sql/paths.sql expected/paths.out
 
 # the db name is hard-coded in the tests
 override USE_MODULE_DB =
diff --git a/contrib/dblink/expected/.gitignore b/contrib/dblink/expected/.gitignore
new file mode 100644 (file)
index 0000000..d9c7942
--- /dev/null
@@ -0,0 +1 @@
+/paths.out
index f237c43d3d42121956f144b449202929cc1fb097..f503691bf218a50fa15f71500c1be9c20af6bddc 100644 (file)
@@ -103,6 +103,33 @@ SELECT *
 FROM dblink('SELECT * FROM foo') AS t(a int, b text, c text[])
 WHERE t.a > 7;
 ERROR:  connection not available
+-- The first-level connection's backend will crash on exit given OpenLDAP
+-- [2.4.24, 2.4.31].  We won't see evidence of any crash until the victim
+-- process terminates and the postmaster responds.  If process termination
+-- entails writing a core dump, that can take awhile.  Wait for the process to
+-- vanish.  At that point, the postmaster has called waitpid() on the crashed
+-- process, and it will accept no new connections until it has reinitialized
+-- the cluster.  (We can't exploit pg_stat_activity, because the crash happens
+-- after the backend updates shared memory to reflect its impending exit.)
+DO $pl$
+DECLARE
+   detail text;
+BEGIN
+   PERFORM wait_pid(crash_pid)
+   FROM dblink('dbname=contrib_regression', $$
+       SELECT pg_backend_pid() FROM dblink(
+           'service=test_ldap dbname=contrib_regression',
+           -- This string concatenation is a hack to shoehorn a
+           -- set_pgservicefile call into the SQL statement.
+           'SELECT 1' || set_pgservicefile('pg_service.conf')
+       ) t(c int)
+   $$) AS t(crash_pid int);
+EXCEPTION WHEN OTHERS THEN
+   GET STACKED DIAGNOSTICS detail = PG_EXCEPTION_DETAIL;
+   -- Expected error in a non-LDAP build.
+   IF NOT detail LIKE 'syntax error in service file%' THEN RAISE; END IF;
+END
+$pl$;
 -- create a persistent connection
 SELECT dblink_connect('dbname=contrib_regression');
  dblink_connect 
diff --git a/contrib/dblink/input/paths.source b/contrib/dblink/input/paths.source
new file mode 100644 (file)
index 0000000..aab3a3b
--- /dev/null
@@ -0,0 +1,14 @@
+-- Initialization that requires path substitution.
+
+CREATE FUNCTION putenv(text)
+   RETURNS void
+   AS '@libdir@/regress@DLSUFFIX@', 'regress_putenv'
+   LANGUAGE C STRICT;
+
+CREATE FUNCTION wait_pid(int)
+   RETURNS void
+   AS '@libdir@/regress@DLSUFFIX@'
+   LANGUAGE C STRICT;
+
+CREATE FUNCTION set_pgservicefile(text) RETURNS void LANGUAGE SQL
+    AS $$SELECT putenv('PGSERVICEFILE=@abs_srcdir@/' || $1)$$;
diff --git a/contrib/dblink/output/paths.source b/contrib/dblink/output/paths.source
new file mode 100644 (file)
index 0000000..e1097f0
--- /dev/null
@@ -0,0 +1,11 @@
+-- Initialization that requires path substitution.
+CREATE FUNCTION putenv(text)
+   RETURNS void
+   AS '@libdir@/regress@DLSUFFIX@', 'regress_putenv'
+   LANGUAGE C STRICT;
+CREATE FUNCTION wait_pid(int)
+   RETURNS void
+   AS '@libdir@/regress@DLSUFFIX@'
+   LANGUAGE C STRICT;
+CREATE FUNCTION set_pgservicefile(text) RETURNS void LANGUAGE SQL
+    AS $$SELECT putenv('PGSERVICEFILE=@abs_srcdir@/' || $1)$$;
diff --git a/contrib/dblink/pg_service.conf b/contrib/dblink/pg_service.conf
new file mode 100644 (file)
index 0000000..92201f0
--- /dev/null
@@ -0,0 +1,7 @@
+# pg_service.conf for minimally exercising libpq use of LDAP.
+
+# Having failed to reach an LDAP server, libpq essentially ignores the
+# "service=test_ldap" in its connection string.  Contact the "discard"
+# service; the test works whether or not it answers.
+[test_ldap]
+ldap://127.0.0.1:9/base?attribute?one?filter
diff --git a/contrib/dblink/sql/.gitignore b/contrib/dblink/sql/.gitignore
new file mode 100644 (file)
index 0000000..d175078
--- /dev/null
@@ -0,0 +1 @@
+/paths.sql
index 2a107601c5565a75153fe47ebd4f05c1c93d7bfa..d8d248260c02d9d6ce10ee258606e3e07ba9b914 100644 (file)
@@ -65,6 +65,34 @@ SELECT *
 FROM dblink('SELECT * FROM foo') AS t(a int, b text, c text[])
 WHERE t.a > 7;
 
+-- The first-level connection's backend will crash on exit given OpenLDAP
+-- [2.4.24, 2.4.31].  We won't see evidence of any crash until the victim
+-- process terminates and the postmaster responds.  If process termination
+-- entails writing a core dump, that can take awhile.  Wait for the process to
+-- vanish.  At that point, the postmaster has called waitpid() on the crashed
+-- process, and it will accept no new connections until it has reinitialized
+-- the cluster.  (We can't exploit pg_stat_activity, because the crash happens
+-- after the backend updates shared memory to reflect its impending exit.)
+DO $pl$
+DECLARE
+   detail text;
+BEGIN
+   PERFORM wait_pid(crash_pid)
+   FROM dblink('dbname=contrib_regression', $$
+       SELECT pg_backend_pid() FROM dblink(
+           'service=test_ldap dbname=contrib_regression',
+           -- This string concatenation is a hack to shoehorn a
+           -- set_pgservicefile call into the SQL statement.
+           'SELECT 1' || set_pgservicefile('pg_service.conf')
+       ) t(c int)
+   $$) AS t(crash_pid int);
+EXCEPTION WHEN OTHERS THEN
+   GET STACKED DIAGNOSTICS detail = PG_EXCEPTION_DETAIL;
+   -- Expected error in a non-LDAP build.
+   IF NOT detail LIKE 'syntax error in service file%' THEN RAISE; END IF;
+END
+$pl$;
+
 -- create a persistent connection
 SELECT dblink_connect('dbname=contrib_regression');
 
index e5136cfa7c89400daa7d2e19b4ea9ece649fa40c..acc5d7a59f271a7e42e20a04b924f3fc4fdbd5c4 100644 (file)
@@ -6,6 +6,7 @@
 
 #include <float.h>
 #include <math.h>
+#include <signal.h>
 
 #include "access/transam.h"
 #include "access/xact.h"
 #include "commands/trigger.h"
 #include "executor/executor.h"
 #include "executor/spi.h"
+#include "miscadmin.h"
 #include "utils/builtins.h"
 #include "utils/geo_decls.h"
+#include "utils/memutils.h"
 #include "utils/rel.h"
 
 
@@ -35,6 +38,8 @@ extern char *reverse_name(char *string);
 extern int oldstyle_length(int n, text *t);
 extern Datum int44in(PG_FUNCTION_ARGS);
 extern Datum int44out(PG_FUNCTION_ARGS);
+extern Datum regress_putenv(PG_FUNCTION_ARGS);
+extern Datum wait_pid(PG_FUNCTION_ARGS);
 
 #ifdef PG_MODULE_MAGIC
 PG_MODULE_MAGIC;
@@ -737,3 +742,44 @@ int44out(PG_FUNCTION_ARGS)
    *--walk = '\0';
    PG_RETURN_CSTRING(result);
 }
+
+PG_FUNCTION_INFO_V1(regress_putenv);
+
+Datum
+regress_putenv(PG_FUNCTION_ARGS)
+{
+   MemoryContext oldcontext;
+   char       *envbuf;
+
+   if (!superuser())
+       elog(ERROR, "must be superuser to change environment variables");
+
+   oldcontext = MemoryContextSwitchTo(TopMemoryContext);
+   envbuf = text_to_cstring((text *) PG_GETARG_POINTER(0));
+   MemoryContextSwitchTo(oldcontext);
+
+   if (putenv(envbuf) != 0)
+       elog(ERROR, "could not set environment variable: %m");
+
+   PG_RETURN_VOID();
+}
+
+/* Sleep until no process has a given PID. */
+PG_FUNCTION_INFO_V1(wait_pid);
+
+Datum
+wait_pid(PG_FUNCTION_ARGS)
+{
+   int         pid = PG_GETARG_INT32(0);
+
+   if (!superuser())
+       elog(ERROR, "must be superuser to check PID liveness");
+
+   while (kill(pid, 0) == 0)
+       pg_usleep(50000);
+
+   if (errno != ESRCH)
+       elog(ERROR, "could not check PID %d liveness: %m", pid);
+
+   PG_RETURN_VOID();
+}