Avoid symbol collisions between pqsignal.c and legacy-pqsignal.c.
authorTom Lane <[email protected]>
Tue, 14 Jan 2025 23:50:24 +0000 (18:50 -0500)
committerTom Lane <[email protected]>
Tue, 14 Jan 2025 23:50:24 +0000 (18:50 -0500)
In the name of ABI stability (that is, to avoid a library major
version bump for libpq), libpq still exports a version of pqsignal()
that we no longer want to use ourselves.  However, since that has
the same link name as the function exported by src/port/pqsignal.c,
there is a link ordering dependency determining which version will
actually get used by code that uses libpq as well as libpgport.a.

It now emerges that the wrong version has been used by pgbench and
psql since commit 06843df4a rearranged their link commands.  This
can result in odd failures in pgbench with the -T switch, since its
SIGALRM handler will now not be marked SA_RESTART.  psql may have
some edge-case problems in \watch, too.

Since we don't want to depend on link ordering effects anymore,
let's fix this in the same spirit as b6c7cfac8: use macros to change
the actual link names of the competing functions.  We cannot change
legacy-pqsignal.c's exported name of course, so the victim has to be
src/port/pqsignal.c.

In master, rename its exported name to be pqsignal_fe in frontend or
pqsignal_be in backend.  (We could perhaps have gotten away with using
the same symbol in both cases, but since the FE and BE versions now
work a little differently, it seems advisable to use different names.)

In back branches, rename to pqsignal_fe in frontend but keep it as
pqsignal in backend.  The frontend change could affect third-party
code that is calling pqsignal from libpgport.a or libpgport_shlib.a,
but only if the code is compiled against port.h from a different minor
release than libpgport.  Since we don't support using libpgport as a
shared library, it seems unlikely that there will be such a problem.
I left the backend symbol unchanged to avoid an ABI break for
extensions.  This means that the link ordering hazard still exists
for any extension that links against libpq.  However, none of our own
extensions use both pqsignal() and libpq, and we're not making things
any worse for third-party extensions that do.

Report from Andy Fan, diagnosis by Fujii Masao,  by me.
Back- to all supported branches, as 06843df4a was.

Discussion: https://postgr.es/m/[email protected]

src/include/port.h
src/interfaces/libpq/legacy-pqsignal.c
src/port/pqsignal.c

index 818b7c7baef7d1c7db0aef362a38b64e22b443c4..f0e28ce5c5360bfe90fadf21cb10b99c24dab9b1 100644 (file)
@@ -513,7 +513,12 @@ extern int pg_check_dir(const char *dir);
 /* port/pgmkdirp.c */
 extern int pg_mkdir_p(char *path, int omode);
 
-/* port/pqsignal.c */
+/* port/pqsignal.c (see also interfaces/libpq/legacy-pqsignal.c) */
+#ifdef FRONTEND
+#define pqsignal pqsignal_fe
+#else
+#define pqsignal pqsignal_be
+#endif
 typedef void (*pqsigfunc) (SIGNAL_ARGS);
 extern pqsigfunc pqsignal(int signo, pqsigfunc func);
 
index 0864888562335b7fee6600e5b983de7b493db8c1..ebd1695bf074faf814b7cd68bdd2f532bb8e59e1 100644 (file)
  * with the semantics it had in 9.2; in particular, this has different
  * behavior for SIGALRM than the version in src/port/pqsignal.c.
  *
- * libpq itself does not use this.
+ * libpq itself does not use this, nor does anything else in our code.
+ *
+ * src/include/port.h #define's pqsignal as pqsignal_fe or pqsignal_be,
+ * but here we want to export just plain "pqsignal".  We can't rely on
+ * port.h's extern declaration either.  (The point of those #define's
+ * is to ensure that no in-tree code accidentally calls this version.)
  */
+#undef pqsignal
+extern pqsigfunc pqsignal(int signo, pqsigfunc func);
+
 pqsigfunc
 pqsignal(int signo, pqsigfunc func)
 {
index d328a27279df8b1fcc60227f67efcf519f5d4a46..1169de6b81e06cb7848ccc678dd9248443c99f70 100644 (file)
@@ -123,6 +123,10 @@ wrapper_handler(SIGNAL_ARGS)
  * function instead of providing potentially-bogus return values.
  * Unfortunately, that requires modifying the pqsignal() in legacy-pqsignal.c,
  * which in turn requires an SONAME bump, which is probably not worth it.
+ *
+ * Note: the actual name of this function is either pqsignal_fe when
+ * compiled with -DFRONTEND, or pqsignal_be when compiled without that.
+ * This is to avoid a name collision with libpq's legacy-pqsignal.c.
  */
 pqsigfunc
 pqsignal(int signo, pqsigfunc func)