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 f587338dec.

Reported-by: Bharath Rupireddy
Reviewed-by: Andrey Borodin
Discussion: https://postgr.es/m/ZhOd3NXAutteokGL@paquier.xyz
This commit is contained in:
Michael Paquier 2024-04-09 10:31:12 +09:00
parent f463de59d9
commit f4083c4975
4 changed files with 35 additions and 0 deletions

View File

@ -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

View File

@ -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;

View File

@ -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,
},

View File

@ -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;