Subject: Re: pgbench error: (setshell) of script 0; execution of meta-command failed

From: Tom Lane <[email-masked]>
Date: Fri, 10 Jan 2025 10:20:51 -0500

I wrote:
> Since legacy-pqsignal is really not supposed to be used by clients
> anymore, maybe we could just adjust it to set SA_RESTART for SIGALRM?
> The other alternatives I can think of amount to re-introducing
> link order dependencies, which would be horrid.

Actually, after re-reading the thread that led to 06843df4a [1],
I think a better idea is to introduce some macro magic to force
frontend clients to use libpgport's version of pqsignal() instead
of the one from libpq.  We mustn't change the real name of libpq's
version, but I think we could get away with that for libpgport.

I'm a bit tied up today, but can look at this over the weekend if
nobody beats me to it.

			regards, tom lane

[1] https://www.postgresql.org/message-id/flat/467042.1695766998%40sss.pgh.pa.us

---

From: Tom Lane <[email-masked]>
Date: Sat, 11 Jan 2025 14:04:13 -0500

> version, but I think we could get away with that for libpgport.

After studying this more, I think what we should do in HEAD is
even more aggressive: let's make the real name of port/pqsignal.c's
function be either pqsignal_fe in frontend, or pqsignal_be in backend.
This positively ensures that there's no collision with libpq's
export, and it seems like a good idea for the additional reason that
the frontend and backend versions are not really interchangeable.

However, we can't get away with that in released branches because
it'd be an ABI break for any outside code that calls pqsignal.
What I propose doing in the released branches is what's shown in
the attached patch for v17: rename port/pqsignal.c's function to
pqsignal_fe in frontend, but leave it as pqsignal in the backend.
Leaving it as pqsignal in the backend preserves ABI for extension
modules, at the cost that it's not entirely clear which pqsignal
an extension that's also linked with libpq will get.  But that
hazard affects none of our code, and it existed already for any
third-party extensions that call pqsignal.  In principle using
"pqsignal_fe" in frontend creates an ABI hazard for outside users of
libpgport.a.  But I think it's not really a problem, because we don't
support that as a shared library.  As long as people build with
headers and .a files from the same minor release, they'll be OK.

BTW, this decouples legacy-pqsignal.c from pqsignal.c enough
that we could now do what's contemplated in the comments from
3b00fdba9: simplify that version by making it return void,
or perhaps better just a true/false success report.
I've not touched that point here, though.

			regards, tom lane

diff --git a/src/include/port.h b/src/include/port.h
index 818b7c7bae..f0e28ce5c5 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -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);
 
diff --git a/src/interfaces/libpq/legacy-pqsignal.c b/src/interfaces/libpq/legacy-pqsignal.c
index 0864888562..01977b4c81 100644
--- a/src/interfaces/libpq/legacy-pqsignal.c
+++ b/src/interfaces/libpq/legacy-pqsignal.c
@@ -28,8 +28,16 @@
  * 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 the #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)
 {
diff --git a/src/port/pqsignal.c b/src/port/pqsignal.c
index d328a27279..f707b1c54e 100644
--- a/src/port/pqsignal.c
+++ b/src/port/pqsignal.c
@@ -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.
+ * This is to avoid a name collision with libpq's legacy-pqsignal.c.
  */
 pqsigfunc
 pqsignal(int signo, pqsigfunc func)
diff --git a/src/include/port.h b/src/include/port.h
index 4d2d911cb6..acc39deac4 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -515,7 +515,10 @@ 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
+#endif
 typedef void (*pqsigfunc) (SIGNAL_ARGS);
 extern pqsigfunc pqsignal(int signo, pqsigfunc func);
 
diff --git a/src/interfaces/libpq/legacy-pqsignal.c b/src/interfaces/libpq/legacy-pqsignal.c
index e8c716ad0f..49755e8acd 100644
--- a/src/interfaces/libpq/legacy-pqsignal.c
+++ b/src/interfaces/libpq/legacy-pqsignal.c
@@ -28,8 +28,16 @@
  * 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 will #define pqsignal as pqsignal_fe,
+ * but here we want to export just plain "pqsignal".  We can't rely on
+ * port.h's extern declaration either.  (The point of the #define
+ * 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)
 {
diff --git a/src/port/pqsignal.c b/src/port/pqsignal.c
index bdaa9f10c8..844e37c827 100644
--- a/src/port/pqsignal.c
+++ b/src/port/pqsignal.c
@@ -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 when compiled without.
+ * This is to avoid a name collision with libpq's legacy-pqsignal.c.
  */
 pqsigfunc
 pqsignal(int signo, pqsigfunc func)