injection_points: Fix race condition with local injection point tests

The module relies on a shmem exit callback to clean up any injection
points linked to a specific process.  One of the tests checks for the
case of an injection point name reused in a second connection where the
first connection should clean it up, but it did not count for the fact
that the shmem exit callback of the first connection may not have run
when the second connection begins its work.

The regress library includes a wait_pid() that can be used for this
purpose, instead of a custom wait logic, so let's rely on it to wait for
the first connection to exit before working with the second connection.
The module gains a REGRESS_OPTS to be able to look at the regress
library's dlpath.

This issue could be reproduced with a hardcoded sleep() in the shmem
exit callback, and the CI has been able to trigger it sporadically.

Oversight in f587338dec87.

Reported-by: Bharath Rupireddy
Reviewed-by: Andrey Borodin
Discussion: https://postgr.es/m/ZhOd3NXAutteokGL@paquier.xyz

4 files changed, 35 insertions, 0 deletions

diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index 1cb395c37ac..31bd787994b 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -7,6 +7,7 @@ DATA = injection_points--1.0.sql
 PGFILEDESC = "injection_points - facility for injection points"
 
 REGRESS = injection_points
+REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
 
 # The injection points are cluster-wide, so disable installcheck
 NO_INSTALLCHECK = 1
diff --git a/src/test/modules/injection_points/expected/injection_points.out b/src/test/modules/injection_points/expected/injection_points.out
index 3d94016ac9c..1341d66c927 100644
--- a/src/test/modules/injection_points/expected/injection_points.out
+++ b/src/test/modules/injection_points/expected/injection_points.out
@@ -1,4 +1,11 @@
 CREATE EXTENSION injection_points;
+\getenv libdir PG_LIBDIR
+\getenv dlsuffix PG_DLSUFFIX
+\set regresslib :libdir '/regress' :dlsuffix
+CREATE FUNCTION wait_pid(int)
+  RETURNS void
+  AS :'regresslib'
+  LANGUAGE C STRICT;
 SELECT injection_points_attach('TestInjectionBooh', 'booh');
 ERROR:  incorrect action "booh" for injection point creation
 SELECT injection_points_attach('TestInjectionError', 'error');
@@ -156,8 +163,17 @@ NOTICE:  notice triggered for injection point TestConditionLocal2
  
 (1 row)
 
+SELECT pg_backend_pid() AS oldpid \gset
 -- reload, local injection points should be gone.
 \c
+-- Wait for the previous backend process to exit, ensuring that its local
+-- injection points are cleaned up.
+SELECT wait_pid(:'oldpid');
+ wait_pid 
+----------
+ 
+(1 row)
+
 SELECT injection_points_run('TestConditionLocal1'); -- nothing
  injection_points_run 
 ----------------------
@@ -193,3 +209,4 @@ SELECT injection_points_detach('TestConditionLocal1');
 (1 row)
 
 DROP EXTENSION injection_points;
+DROP FUNCTION wait_pid;
diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build
index a29217f75f9..8e1b5b45391 100644
--- a/src/test/modules/injection_points/meson.build
+++ b/src/test/modules/injection_points/meson.build
@@ -33,6 +33,7 @@ tests += {
     'sql': [
       'injection_points',
     ],
+    'regress_args': ['--dlpath', meson.build_root() / 'src/test/regress'],
     # The injection points are cluster-wide, so disable installcheck
     'runningcheck': false,
   },
diff --git a/src/test/modules/injection_points/sql/injection_points.sql b/src/test/modules/injection_points/sql/injection_points.sql
index 2aa76a542bb..71e2972a7e4 100644
--- a/src/test/modules/injection_points/sql/injection_points.sql
+++ b/src/test/modules/injection_points/sql/injection_points.sql
@@ -1,5 +1,14 @@
 CREATE EXTENSION injection_points;
 
+\getenv libdir PG_LIBDIR
+\getenv dlsuffix PG_DLSUFFIX
+\set regresslib :libdir '/regress' :dlsuffix
+
+CREATE FUNCTION wait_pid(int)
+  RETURNS void
+  AS :'regresslib'
+  LANGUAGE C STRICT;
+
 SELECT injection_points_attach('TestInjectionBooh', 'booh');
 SELECT injection_points_attach('TestInjectionError', 'error');
 SELECT injection_points_attach('TestInjectionLog', 'notice');
@@ -40,8 +49,14 @@ SELECT injection_points_attach('TestConditionLocal1', 'error');
 SELECT injection_points_attach('TestConditionLocal2', 'notice');
 SELECT injection_points_run('TestConditionLocal1'); -- error
 SELECT injection_points_run('TestConditionLocal2'); -- notice
+
+SELECT pg_backend_pid() AS oldpid \gset
+
 -- reload, local injection points should be gone.
 \c
+-- Wait for the previous backend process to exit, ensuring that its local
+-- injection points are cleaned up.
+SELECT wait_pid(:'oldpid');
 SELECT injection_points_run('TestConditionLocal1'); -- nothing
 SELECT injection_points_run('TestConditionLocal2'); -- nothing
 SELECT injection_points_run('TestConditionError'); -- error
@@ -52,3 +67,4 @@ SELECT injection_points_attach('TestConditionLocal1', 'error');
 SELECT injection_points_detach('TestConditionLocal1');
 
 DROP EXTENSION injection_points;
+DROP FUNCTION wait_pid;