Skip to content

Commit 4616d57

Browse files
tglsfdctglsfdc
committed
Fix all the server-side SIGQUIT handlers (grumble ... why so many identical
copies?) to ensure they really don't run proc_exit/shmem_exit callbacks, as was intended. I broke this behavior recently by installing atexit callbacks without thinking about the one case where we truly don't want to run those callback functions. Noted in an example from Dave Page.
1 parent abc9245 commit 4616d57

File tree

6 files changed

+76
-34
lines changed

6 files changed

+76
-34
lines changed

‎src/backend/access/transam/xlog.c

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/backend/access/transam/xlog.c,v 1.339 2009/05/14 21:28:35 tgl Exp $
10+
* $PostgreSQL: pgsql/src/backend/access/transam/xlog.c,v 1.340 2009/05/15 15:56:39 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -7790,14 +7790,22 @@ startupproc_quickdie(SIGNAL_ARGS)
77907790
PG_SETMASK(&BlockSig);
77917791

77927792
/*
7793-
* DO NOT proc_exit() -- we're here because shared memory may be
7794-
* corrupted, so we don't want to try to clean up our transaction. Just
7795-
* nail the windows shut and get out of town.
7796-
*
7793+
* We DO NOT want to run proc_exit() callbacks -- we're here because
7794+
* shared memory may be corrupted, so we don't want to try to clean up our
7795+
* transaction. Just nail the windows shut and get out of town. Now that
7796+
* there's an atexit callback to prevent third-party code from breaking
7797+
* things by calling exit() directly, we have to reset the callbacks
7798+
* explicitly to make this work as intended.
7799+
*/
7800+
on_exit_reset();
7801+
7802+
/*
77977803
* Note we do exit(2) not exit(0). This is to force the postmaster into a
77987804
* system reset cycle if some idiot DBA sends a manual SIGQUIT to a random
77997805
* backend. This is necessary precisely because we don't clean up our
7800-
* shared memory state.
7806+
* shared memory state. (The "dead man switch" mechanism in pmsignal.c
7807+
* should ensure the postmaster sees this as a crash, too, but no harm
7808+
* in being doubly sure.)
78017809
*/
78027810
exit(2);
78037811
}

‎src/backend/postmaster/autovacuum.c

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@
5555
*
5656
*
5757
* IDENTIFICATION
58-
* $PostgreSQL: pgsql/src/backend/postmaster/autovacuum.c,v 1.94 2009/03/31 22:12:48 tgl Exp $
58+
* $PostgreSQL: pgsql/src/backend/postmaster/autovacuum.c,v 1.95 2009/05/15 15:56:39 tgl Exp $
5959
*
6060
*-------------------------------------------------------------------------
6161
*/
@@ -1337,14 +1337,22 @@ avl_quickdie(SIGNAL_ARGS)
13371337
PG_SETMASK(&BlockSig);
13381338

13391339
/*
1340-
* DO NOT proc_exit() -- we're here because shared memory may be
1341-
* corrupted, so we don't want to try to clean up our transaction. Just
1342-
* nail the windows shut and get out of town.
1343-
*
1340+
* We DO NOT want to run proc_exit() callbacks -- we're here because
1341+
* shared memory may be corrupted, so we don't want to try to clean up our
1342+
* transaction. Just nail the windows shut and get out of town. Now that
1343+
* there's an atexit callback to prevent third-party code from breaking
1344+
* things by calling exit() directly, we have to reset the callbacks
1345+
* explicitly to make this work as intended.
1346+
*/
1347+
on_exit_reset();
1348+
1349+
/*
13441350
* Note we do exit(2) not exit(0). This is to force the postmaster into a
13451351
* system reset cycle if some idiot DBA sends a manual SIGQUIT to a random
13461352
* backend. This is necessary precisely because we don't clean up our
1347-
* shared memory state.
1353+
* shared memory state. (The "dead man switch" mechanism in pmsignal.c
1354+
* should ensure the postmaster sees this as a crash, too, but no harm
1355+
* in being doubly sure.)
13481356
*/
13491357
exit(2);
13501358
}

‎src/backend/postmaster/bgwriter.c

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
*
3838
*
3939
* IDENTIFICATION
40-
* $PostgreSQL: pgsql/src/backend/postmaster/bgwriter.c,v 1.57 2009/03/26 22:26:06 petere Exp $
40+
* $PostgreSQL: pgsql/src/backend/postmaster/bgwriter.c,v 1.58 2009/05/15 15:56:39 tgl Exp $
4141
*
4242
*-------------------------------------------------------------------------
4343
*/
@@ -798,14 +798,22 @@ bg_quickdie(SIGNAL_ARGS)
798798
PG_SETMASK(&BlockSig);
799799

800800
/*
801-
* DO NOT proc_exit() -- we're here because shared memory may be
802-
* corrupted, so we don't want to try to clean up our transaction. Just
803-
* nail the windows shut and get out of town.
804-
*
801+
* We DO NOT want to run proc_exit() callbacks -- we're here because
802+
* shared memory may be corrupted, so we don't want to try to clean up our
803+
* transaction. Just nail the windows shut and get out of town. Now that
804+
* there's an atexit callback to prevent third-party code from breaking
805+
* things by calling exit() directly, we have to reset the callbacks
806+
* explicitly to make this work as intended.
807+
*/
808+
on_exit_reset();
809+
810+
/*
805811
* Note we do exit(2) not exit(0). This is to force the postmaster into a
806812
* system reset cycle if some idiot DBA sends a manual SIGQUIT to a random
807813
* backend. This is necessary precisely because we don't clean up our
808-
* shared memory state.
814+
* shared memory state. (The "dead man switch" mechanism in pmsignal.c
815+
* should ensure the postmaster sees this as a crash, too, but no harm
816+
* in being doubly sure.)
809817
*/
810818
exit(2);
811819
}

‎src/backend/postmaster/walwriter.c

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
*
3535
*
3636
* IDENTIFICATION
37-
* $PostgreSQL: pgsql/src/backend/postmaster/walwriter.c,v 1.5 2009/01/01 17:23:46 momjian Exp $
37+
* $PostgreSQL: pgsql/src/backend/postmaster/walwriter.c,v 1.6 2009/05/15 15:56:39 tgl Exp $
3838
*
3939
*-------------------------------------------------------------------------
4040
*/
@@ -288,14 +288,22 @@ wal_quickdie(SIGNAL_ARGS)
288288
PG_SETMASK(&BlockSig);
289289

290290
/*
291-
* DO NOT proc_exit() -- we're here because shared memory may be
292-
* corrupted, so we don't want to try to clean up our transaction. Just
293-
* nail the windows shut and get out of town.
294-
*
291+
* We DO NOT want to run proc_exit() callbacks -- we're here because
292+
* shared memory may be corrupted, so we don't want to try to clean up our
293+
* transaction. Just nail the windows shut and get out of town. Now that
294+
* there's an atexit callback to prevent third-party code from breaking
295+
* things by calling exit() directly, we have to reset the callbacks
296+
* explicitly to make this work as intended.
297+
*/
298+
on_exit_reset();
299+
300+
/*
295301
* Note we do exit(2) not exit(0). This is to force the postmaster into a
296302
* system reset cycle if some idiot DBA sends a manual SIGQUIT to a random
297303
* backend. This is necessary precisely because we don't clean up our
298-
* shared memory state.
304+
* shared memory state. (The "dead man switch" mechanism in pmsignal.c
305+
* should ensure the postmaster sees this as a crash, too, but no harm
306+
* in being doubly sure.)
299307
*/
300308
exit(2);
301309
}

‎src/backend/storage/ipc/ipc.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
*
1414
*
1515
* IDENTIFICATION
16-
* $PostgreSQL: pgsql/src/backend/storage/ipc/ipc.c,v 1.103 2009/05/05 20:06:07 tgl Exp $
16+
* $PostgreSQL: pgsql/src/backend/storage/ipc/ipc.c,v 1.104 2009/05/15 15:56:39 tgl Exp $
1717
*
1818
*-------------------------------------------------------------------------
1919
*/
@@ -166,7 +166,8 @@ proc_exit_prepare(int code)
166166
/* do our shared memory exits first */
167167
shmem_exit(code);
168168

169-
elog(DEBUG3, "proc_exit(%d)", code);
169+
elog(DEBUG3, "proc_exit(%d): %d callbacks to make",
170+
code, on_proc_exit_index);
170171

171172
/*
172173
* call all the registered callbacks.
@@ -193,7 +194,8 @@ proc_exit_prepare(int code)
193194
void
194195
shmem_exit(int code)
195196
{
196-
elog(DEBUG3, "shmem_exit(%d)", code);
197+
elog(DEBUG3, "shmem_exit(%d): %d callbacks to make",
198+
code, on_shmem_exit_index);
197199

198200
/*
199201
* call all the registered callbacks.

‎src/backend/tcop/postgres.c

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.565 2009/01/07 19:35:43 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.566 2009/05/15 15:56:39 tgl Exp $
1212
*
1313
* NOTES
1414
* this is the "main" module of the postgres backend and
@@ -2495,14 +2495,22 @@ quickdie(SIGNAL_ARGS)
24952495
" database and repeat your command.")));
24962496

24972497
/*
2498-
* DO NOT proc_exit() -- we're here because shared memory may be
2499-
* corrupted, so we don't want to try to clean up our transaction. Just
2500-
* nail the windows shut and get out of town.
2501-
*
2498+
* We DO NOT want to run proc_exit() callbacks -- we're here because
2499+
* shared memory may be corrupted, so we don't want to try to clean up our
2500+
* transaction. Just nail the windows shut and get out of town. Now that
2501+
* there's an atexit callback to prevent third-party code from breaking
2502+
* things by calling exit() directly, we have to reset the callbacks
2503+
* explicitly to make this work as intended.
2504+
*/
2505+
on_exit_reset();
2506+
2507+
/*
25022508
* Note we do exit(2) not exit(0). This is to force the postmaster into a
25032509
* system reset cycle if some idiot DBA sends a manual SIGQUIT to a random
25042510
* backend. This is necessary precisely because we don't clean up our
2505-
* shared memory state.
2511+
* shared memory state. (The "dead man switch" mechanism in pmsignal.c
2512+
* should ensure the postmaster sees this as a crash, too, but no harm
2513+
* in being doubly sure.)
25062514
*/
25072515
exit(2);
25082516
}
@@ -2554,7 +2562,7 @@ die(SIGNAL_ARGS)
25542562
void
25552563
authdie(SIGNAL_ARGS)
25562564
{
2557-
exit(1);
2565+
proc_exit(1);
25582566
}
25592567

25602568
/*

0 commit comments

Comments
 (0)