Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit f4083c4

Browse filesBrowse files
michaelpqmichaelpq
committed
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 f587338. Reported-by: Bharath Rupireddy Reviewed-by: Andrey Borodin Discussion: https://postgr.es/m/ZhOd3NXAutteokGL@paquier.xyz
1 parent f463de5 commit f4083c4
Copy full SHA for f4083c4

File tree

Expand file treeCollapse file tree

4 files changed

+35
-0
lines changed
Filter options
Expand file treeCollapse file tree

4 files changed

+35
-0
lines changed

‎src/test/modules/injection_points/Makefile

Copy file name to clipboardExpand all lines: src/test/modules/injection_points/Makefile
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ DATA = injection_points--1.0.sql
77
PGFILEDESC = "injection_points - facility for injection points"
88

99
REGRESS = injection_points
10+
REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
1011

1112
# The injection points are cluster-wide, so disable installcheck
1213
NO_INSTALLCHECK = 1

‎src/test/modules/injection_points/expected/injection_points.out

Copy file name to clipboardExpand all lines: src/test/modules/injection_points/expected/injection_points.out
+17Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,11 @@
11
CREATE EXTENSION injection_points;
2+
\getenv libdir PG_LIBDIR
3+
\getenv dlsuffix PG_DLSUFFIX
4+
\set regresslib :libdir '/regress' :dlsuffix
5+
CREATE FUNCTION wait_pid(int)
6+
RETURNS void
7+
AS :'regresslib'
8+
LANGUAGE C STRICT;
29
SELECT injection_points_attach('TestInjectionBooh', 'booh');
310
ERROR: incorrect action "booh" for injection point creation
411
SELECT injection_points_attach('TestInjectionError', 'error');
@@ -156,8 +163,17 @@ NOTICE: notice triggered for injection point TestConditionLocal2
156163

157164
(1 row)
158165

166+
SELECT pg_backend_pid() AS oldpid \gset
159167
-- reload, local injection points should be gone.
160168
\c
169+
-- Wait for the previous backend process to exit, ensuring that its local
170+
-- injection points are cleaned up.
171+
SELECT wait_pid(:'oldpid');
172+
wait_pid
173+
----------
174+
175+
(1 row)
176+
161177
SELECT injection_points_run('TestConditionLocal1'); -- nothing
162178
injection_points_run
163179
----------------------
@@ -193,3 +209,4 @@ SELECT injection_points_detach('TestConditionLocal1');
193209
(1 row)
194210

195211
DROP EXTENSION injection_points;
212+
DROP FUNCTION wait_pid;

‎src/test/modules/injection_points/meson.build

Copy file name to clipboardExpand all lines: src/test/modules/injection_points/meson.build
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ tests += {
3333
'sql': [
3434
'injection_points',
3535
],
36+
'regress_args': ['--dlpath', meson.build_root() / 'src/test/regress'],
3637
# The injection points are cluster-wide, so disable installcheck
3738
'runningcheck': false,
3839
},

‎src/test/modules/injection_points/sql/injection_points.sql

Copy file name to clipboardExpand all lines: src/test/modules/injection_points/sql/injection_points.sql
+16Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
11
CREATE EXTENSION injection_points;
22

3+
\getenv libdir PG_LIBDIR
4+
\getenv dlsuffix PG_DLSUFFIX
5+
\set regresslib :libdir '/regress' :dlsuffix
6+
7+
CREATE FUNCTION wait_pid(int)
8+
RETURNS void
9+
AS :'regresslib'
10+
LANGUAGE C STRICT;
11+
312
SELECT injection_points_attach('TestInjectionBooh', 'booh');
413
SELECT injection_points_attach('TestInjectionError', 'error');
514
SELECT injection_points_attach('TestInjectionLog', 'notice');
@@ -40,8 +49,14 @@ SELECT injection_points_attach('TestConditionLocal1', 'error');
4049
SELECT injection_points_attach('TestConditionLocal2', 'notice');
4150
SELECT injection_points_run('TestConditionLocal1'); -- error
4251
SELECT injection_points_run('TestConditionLocal2'); -- notice
52+
53+
SELECT pg_backend_pid() AS oldpid \gset
54+
4355
-- reload, local injection points should be gone.
4456
\c
57+
-- Wait for the previous backend process to exit, ensuring that its local
58+
-- injection points are cleaned up.
59+
SELECT wait_pid(:'oldpid');
4560
SELECT injection_points_run('TestConditionLocal1'); -- nothing
4661
SELECT injection_points_run('TestConditionLocal2'); -- nothing
4762
SELECT injection_points_run('TestConditionError'); -- error
@@ -52,3 +67,4 @@ SELECT injection_points_attach('TestConditionLocal1', 'error');
5267
SELECT injection_points_detach('TestConditionLocal1');
5368

5469
DROP EXTENSION injection_points;
70+
DROP FUNCTION wait_pid;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.