Hello, Heikki, Nathan and Michael!
Oh, please excuse my impudence in bringing you all here, but I finally
found what almost the same issue was fixed by Heikki already [0].
I discovered that a similar issue was previously addressed by Heikki in
commit [0], where installcheck was disabled for injection point tests.
However, in the meson build configuration, this was only applied to
regression tests - the isolation and TAP tests are still running during
installcheck.
As demonstrated in the previously shared reproducer [1], even *local*
injection points can cause backend crashes through unexpected side effects.
Therefore, I propose extending the installcheck disable to cover both TAP
and isolation tests as well.
I've attached a patch implementing these changes.
A patch with such change is attached.
Best regards,
Mikhail.
[0]:
https://github.com/postgres/postgres/commit/e2e3b8ae9ed73fcd3096c5ca93971891a7767388
[1]:
https://www.postgresql.org/message-id/flat/CANtu0ojbx6%3DesP8euQgzD1CN6tigTQvDmupwEmLTHZT%3D6_yx_A%40mail.gmail.com#18544d553544da67b4fc1ef764df3c3d
>
From e56b6de29aea01916d35d22e9a59241a04202b37 Mon Sep 17 00:00:00 2001
From: nkey <michail.nikol...@gmail.com>
Date: Wed, 27 Nov 2024 02:10:17 +0100
Subject: [PATCH v2] Test to reproduce issue with crash caused by passing throw
injection points during transaction aborting (caused by call to
injection_init_shmem).
---
src/backend/utils/time/snapmgr.c | 2 +
src/test/modules/injection_points/Makefile | 2 +-
.../injection_points/expected/crash.out | 26 ++++++++++
src/test/modules/injection_points/meson.build | 1 +
.../modules/injection_points/specs/crash.spec | 47 +++++++++++++++++++
5 files changed, 77 insertions(+), 1 deletion(-)
create mode 100644 src/test/modules/injection_points/expected/crash.out
create mode 100644 src/test/modules/injection_points/specs/crash.spec
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 7d2b34d4f20..3a7357a050d 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -64,6 +64,7 @@
#include "utils/resowner.h"
#include "utils/snapmgr.h"
#include "utils/syscache.h"
+#include "utils/injection_point.h"
/*
@@ -426,6 +427,7 @@ InvalidateCatalogSnapshot(void)
pairingheap_remove(&RegisteredSnapshots,
&CatalogSnapshot->ph_node);
CatalogSnapshot = NULL;
SnapshotResetXmin();
+ INJECTION_POINT("invalidate_catalog_snapshot_end");
}
}
diff --git a/src/test/modules/injection_points/Makefile
b/src/test/modules/injection_points/Makefile
index 0753a9df58c..da8930ea49f 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -13,7 +13,7 @@ PGFILEDESC = "injection_points - facility for injection
points"
REGRESS = injection_points reindex_conc
REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
-ISOLATION = basic inplace
+ISOLATION = basic crash inplace
TAP_TESTS = 1
diff --git a/src/test/modules/injection_points/expected/crash.out
b/src/test/modules/injection_points/expected/crash.out
new file mode 100644
index 00000000000..7d7f298c786
--- /dev/null
+++ b/src/test/modules/injection_points/expected/crash.out
@@ -0,0 +1,26 @@
+Parsed test spec with 4 sessions
+
+starting permutation: s4_attach_locally wx1 rxwy2 c1 ry3 c2 c3
+injection_points_set_local
+--------------------------
+
+(1 row)
+
+step s4_attach_locally: SELECT
injection_points_attach('invalidate_catalog_snapshot_end', 'wait');
+injection_points_attach
+-----------------------
+
+(1 row)
+
+step wx1: update D1 set id = id + 1;
+step rxwy2: update D2 set id = (select id+1 from D1);
+step c1: COMMIT;
+step ry3: select id from D2;
+id
+--
+ 1
+(1 row)
+
+step c2: COMMIT;
+ERROR: could not serialize access due to read/write dependencies among
transactions
+step c3: COMMIT;
diff --git a/src/test/modules/injection_points/meson.build
b/src/test/modules/injection_points/meson.build
index 58f19001157..6079187dd57 100644
--- a/src/test/modules/injection_points/meson.build
+++ b/src/test/modules/injection_points/meson.build
@@ -43,6 +43,7 @@ tests += {
'isolation': {
'specs': [
'basic',
+ 'crash',
'inplace',
],
},
diff --git a/src/test/modules/injection_points/specs/crash.spec
b/src/test/modules/injection_points/specs/crash.spec
new file mode 100644
index 00000000000..55e6a5434ab
--- /dev/null
+++ b/src/test/modules/injection_points/specs/crash.spec
@@ -0,0 +1,47 @@
+setup
+{
+ create table D1 (id int primary key not null);
+ create table D2 (id int primary key not null);
+ insert into D1 values (1);
+ insert into D2 values (1);
+
+ CREATE EXTENSION injection_points;
+}
+
+teardown
+{
+ DROP TABLE D1, D2;
+ DROP EXTENSION injection_points;
+}
+
+session s1
+setup { BEGIN ISOLATION LEVEL SERIALIZABLE;}
+step wx1 { update D1 set id = id + 1; }
+step c1 { COMMIT; }
+
+session s2
+setup {
+ BEGIN ISOLATION LEVEL SERIALIZABLE;
+}
+step rxwy2 { update D2 set id = (select id+1 from D1); }
+step c2 { COMMIT; }
+
+session s3
+setup { BEGIN ISOLATION LEVEL SERIALIZABLE; }
+step ry3 { select id from D2; }
+step c3 { COMMIT; }
+
+session s4
+setup {
+ SELECT injection_points_set_local();
+}
+step s4_attach_locally { SELECT
injection_points_attach('invalidate_catalog_snapshot_end', 'wait'); }
+
+permutation
+ s4_attach_locally
+ wx1
+ rxwy2
+ c1
+ ry3
+ c2
+ c3
\ No newline at end of file
--
2.43.0
