diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c index 80db5170162..5cc42e9abc1 100644 --- a/src/backend/access/nbtree/nbtree.c +++ b/src/backend/access/nbtree/nbtree.c @@ -12,7 +12,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/nbtree/nbtree.c,v 1.153 2006/11/01 19:43:17 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/access/nbtree/nbtree.c,v 1.153.2.1 2008/04/17 00:00:00 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -23,6 +23,7 @@ #include "catalog/index.h" #include "commands/vacuum.h" #include "storage/freespace.h" +#include "storage/ipc.h" #include "storage/lmgr.h" #include "utils/memutils.h" @@ -538,21 +539,15 @@ btbulkdelete(PG_FUNCTION_ARGS) stats = (IndexBulkDeleteResult *) palloc0(sizeof(IndexBulkDeleteResult)); /* Establish the vacuum cycle ID to use for this scan */ - PG_TRY(); + /* The ENSURE stuff ensures we clean up shared memory on failure */ + PG_ENSURE_ERROR_CLEANUP(_bt_end_vacuum_callback, PointerGetDatum(rel)); { cycleid = _bt_start_vacuum(rel); btvacuumscan(info, stats, callback, callback_state, cycleid); - - _bt_end_vacuum(rel); } - PG_CATCH(); - { - /* Make sure shared memory gets cleaned up */ - _bt_end_vacuum(rel); - PG_RE_THROW(); - } - PG_END_TRY(); + PG_END_ENSURE_ERROR_CLEANUP(_bt_end_vacuum_callback, PointerGetDatum(rel)); + _bt_end_vacuum(rel); PG_RETURN_POINTER(stats); } diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c index 47abb5750dd..0ea2df817c0 100644 --- a/src/backend/access/nbtree/nbtutils.c +++ b/src/backend/access/nbtree/nbtutils.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/nbtree/nbtutils.c,v 1.79.2.1 2007/03/30 00:13:05 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/access/nbtree/nbtutils.c,v 1.79.2.2 2008/04/17 00:00:00 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -974,8 +974,11 @@ _bt_vacuum_cycleid(Relation rel) /* * _bt_start_vacuum --- assign a cycle ID to a just-starting VACUUM operation * - * Note: the caller must guarantee (via PG_TRY) that it will eventually call - * _bt_end_vacuum, else we'll permanently leak an array slot. + * Note: the caller must guarantee that it will eventually call + * _bt_end_vacuum, else we'll permanently leak an array slot. To ensure + * that this happens even in elog(FATAL) scenarios, the appropriate coding + * is not just a PG_TRY, but + * PG_ENSURE_ERROR_CLEANUP(_bt_end_vacuum_callback, PointerGetDatum(rel)) */ BTCycleId _bt_start_vacuum(Relation rel) @@ -1057,6 +1060,15 @@ _bt_end_vacuum(Relation rel) LWLockRelease(BtreeVacuumLock); } +/* + * _bt_end_vacuum wrapped as an on_shmem_exit callback function + */ +void +_bt_end_vacuum_callback(int code, Datum arg) +{ + _bt_end_vacuum((Relation) DatumGetPointer(arg)); +} + /* * BTreeShmemSize --- report amount of shared memory space needed */ diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 63500b304a8..3f9383e8697 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/backend/access/transam/xlog.c,v 1.258.2.2 2007/09/29 01:36:19 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/access/transam/xlog.c,v 1.258.2.3 2008/04/17 00:00:00 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -41,6 +41,7 @@ #include "postmaster/bgwriter.h" #include "storage/bufpage.h" #include "storage/fd.h" +#include "storage/ipc.h" #include "storage/pmsignal.h" #include "storage/procarray.h" #include "storage/spin.h" @@ -501,11 +502,11 @@ static void writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI, static void WriteControlFile(void); static void ReadControlFile(void); static char *str_time(time_t tnow); -static void issue_xlog_fsync(void); - #ifdef WAL_DEBUG static void xlog_outrec(StringInfo buf, XLogRecord *record); #endif +static void issue_xlog_fsync(void); +static void pg_start_backup_callback(int code, Datum arg); static bool read_backup_label(XLogRecPtr *checkPointLoc, XLogRecPtr *minRecoveryLoc); static void rm_redo_error_callback(void *arg); @@ -6185,8 +6186,8 @@ pg_start_backup(PG_FUNCTION_ARGS) XLogCtl->Insert.forcePageWrites = true; LWLockRelease(WALInsertLock); - /* Use a TRY block to ensure we release forcePageWrites if fail below */ - PG_TRY(); + /* Ensure we release forcePageWrites if fail below */ + PG_ENSURE_ERROR_CLEANUP(pg_start_backup_callback, (Datum) 0); { /* * Force a CHECKPOINT. Aside from being necessary to prevent torn @@ -6261,16 +6262,7 @@ pg_start_backup(PG_FUNCTION_ARGS) errmsg("could not write file \"%s\": %m", BACKUP_LABEL_FILE))); } - PG_CATCH(); - { - /* Turn off forcePageWrites on failure */ - LWLockAcquire(WALInsertLock, LW_EXCLUSIVE); - XLogCtl->Insert.forcePageWrites = false; - LWLockRelease(WALInsertLock); - - PG_RE_THROW(); - } - PG_END_TRY(); + PG_END_ENSURE_ERROR_CLEANUP(pg_start_backup_callback, (Datum) 0); /* * We're done. As a convenience, return the starting WAL location. @@ -6282,6 +6274,16 @@ pg_start_backup(PG_FUNCTION_ARGS) PG_RETURN_TEXT_P(result); } +/* Error cleanup callback for pg_start_backup */ +static void +pg_start_backup_callback(int code, Datum arg) +{ + /* Turn off forcePageWrites on failure */ + LWLockAcquire(WALInsertLock, LW_EXCLUSIVE); + XLogCtl->Insert.forcePageWrites = false; + LWLockRelease(WALInsertLock); +} + /* * pg_stop_backup: finish taking an on-line backup dump * diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index fdd5e32b37b..13bb4f92c22 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -13,7 +13,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/dbcommands.c,v 1.187.2.2 2007/04/12 15:04:41 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/dbcommands.c,v 1.187.2.3 2008/04/17 00:00:00 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -39,6 +39,7 @@ #include "miscadmin.h" #include "postmaster/bgwriter.h" #include "storage/freespace.h" +#include "storage/ipc.h" #include "storage/procarray.h" #include "storage/smgr.h" #include "utils/acl.h" @@ -50,7 +51,14 @@ #include "utils/syscache.h" +typedef struct +{ + Oid src_dboid; /* source (template) DB */ + Oid dest_dboid; /* DB we are trying to create */ +} createdb_failure_params; + /* non-export function prototypes */ +static void createdb_failure_callback(int code, Datum arg); static bool get_db_info(const char *name, LOCKMODE lockmode, Oid *dbIdP, Oid *ownerIdP, int *encodingP, bool *dbIsTemplateP, bool *dbAllowConnP, @@ -95,6 +103,7 @@ createdb(const CreatedbStmt *stmt) const char *dbtemplate = NULL; int encoding = -1; int dbconnlimit = -1; + createdb_failure_params fparms; /* don't call this in a transaction block */ PreventTransactionChain((void *) stmt, "CREATE DATABASE"); @@ -406,12 +415,15 @@ createdb(const CreatedbStmt *stmt) /* * Once we start copying subdirectories, we need to be able to clean 'em - * up if we fail. Establish a TRY block to make sure this happens. (This + * up if we fail. Use an ENSURE block to make sure this happens. (This * is not a 100% solution, because of the possibility of failure during * transaction commit after we leave this routine, but it should handle * most scenarios.) */ - PG_TRY(); + fparms.src_dboid = src_dboid; + fparms.dest_dboid = dboid; + PG_ENSURE_ERROR_CLEANUP(createdb_failure_callback, + PointerGetDatum(&fparms)); { /* * Iterate through all tablespaces of the template database, and copy @@ -518,18 +530,25 @@ createdb(const CreatedbStmt *stmt) */ database_file_update_needed(); } - PG_CATCH(); - { - /* Release lock on source database before doing recursive remove */ - UnlockSharedObject(DatabaseRelationId, src_dboid, 0, - ShareLock); + PG_END_ENSURE_ERROR_CLEANUP(createdb_failure_callback, + PointerGetDatum(&fparms)); +} - /* Throw away any successfully copied subdirectories */ - remove_dbtablespaces(dboid); +/* Error cleanup callback for createdb */ +static void +createdb_failure_callback(int code, Datum arg) +{ + createdb_failure_params *fparms = (createdb_failure_params *) DatumGetPointer(arg); - PG_RE_THROW(); - } - PG_END_TRY(); + /* + * Release lock on source database before doing recursive remove. + * This is not essential but it seems desirable to release the lock + * as soon as possible. + */ + UnlockSharedObject(DatabaseRelationId, fparms->src_dboid, 0, ShareLock); + + /* Throw away any successfully copied subdirectories */ + remove_dbtablespaces(fparms->dest_dboid); } diff --git a/src/backend/port/ipc_test.c b/src/backend/port/ipc_test.c index 46d507ccce7..b746fe33d7d 100644 --- a/src/backend/port/ipc_test.c +++ b/src/backend/port/ipc_test.c @@ -21,7 +21,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/port/ipc_test.c,v 1.21 2006/07/14 05:28:28 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/port/ipc_test.c,v 1.21.2.1 2008/04/17 00:00:00 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -58,7 +58,7 @@ char *DataDir = "."; static struct ONEXIT { - void (*function) (int code, Datum arg); + pg_on_exit_callback function; Datum arg; } on_proc_exit_list[MAX_ON_EXITS], on_shmem_exit_list[MAX_ON_EXITS]; @@ -85,7 +85,7 @@ shmem_exit(int code) } void - on_shmem_exit(void (*function) (int code, Datum arg), Datum arg) +on_shmem_exit(pg_on_exit_callback function, Datum arg) { if (on_shmem_exit_index >= MAX_ON_EXITS) elog(FATAL, "out of on_shmem_exit slots"); diff --git a/src/backend/storage/ipc/ipc.c b/src/backend/storage/ipc/ipc.c index 515e2da067e..2195f57cfef 100644 --- a/src/backend/storage/ipc/ipc.c +++ b/src/backend/storage/ipc/ipc.c @@ -13,7 +13,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/storage/ipc/ipc.c,v 1.94 2006/07/14 05:28:28 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/storage/ipc/ipc.c,v 1.94.2.1 2008/04/17 00:00:01 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -52,7 +52,7 @@ bool proc_exit_inprogress = false; static struct ONEXIT { - void (*function) (int code, Datum arg); + pg_on_exit_callback function; Datum arg; } on_proc_exit_list[MAX_ON_EXITS], on_shmem_exit_list[MAX_ON_EXITS]; @@ -145,7 +145,7 @@ shmem_exit(int code) * ---------------------------------------------------------------- */ void - on_proc_exit(void (*function) (int code, Datum arg), Datum arg) +on_proc_exit(pg_on_exit_callback function, Datum arg) { if (on_proc_exit_index >= MAX_ON_EXITS) ereport(FATAL, @@ -166,7 +166,7 @@ void * ---------------------------------------------------------------- */ void - on_shmem_exit(void (*function) (int code, Datum arg), Datum arg) +on_shmem_exit(pg_on_exit_callback function, Datum arg) { if (on_shmem_exit_index >= MAX_ON_EXITS) ereport(FATAL, @@ -179,6 +179,24 @@ void ++on_shmem_exit_index; } +/* ---------------------------------------------------------------- + * cancel_shmem_exit + * + * this function removes an entry, if present, from the list of + * functions to be invoked by shmem_exit(). For simplicity, + * only the latest entry can be removed. (We could work harder + * but there is no need for current uses.) + * ---------------------------------------------------------------- + */ +void +cancel_shmem_exit(pg_on_exit_callback function, Datum arg) +{ + if (on_shmem_exit_index > 0 && + on_shmem_exit_list[on_shmem_exit_index - 1].function == function && + on_shmem_exit_list[on_shmem_exit_index - 1].arg == arg) + --on_shmem_exit_index; +} + /* ---------------------------------------------------------------- * on_exit_reset * diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h index 2e3be26a738..6bedd497603 100644 --- a/src/include/access/nbtree.h +++ b/src/include/access/nbtree.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/access/nbtree.h,v 1.106 2006/11/01 19:43:17 tgl Exp $ + * $PostgreSQL: pgsql/src/include/access/nbtree.h,v 1.106.2.1 2008/04/17 00:00:01 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -538,6 +538,7 @@ extern void _bt_killitems(IndexScanDesc scan, bool haveLock); extern BTCycleId _bt_vacuum_cycleid(Relation rel); extern BTCycleId _bt_start_vacuum(Relation rel); extern void _bt_end_vacuum(Relation rel); +extern void _bt_end_vacuum_callback(int code, Datum arg); extern Size BTreeShmemSize(void); extern void BTreeShmemInit(void); diff --git a/src/include/storage/ipc.h b/src/include/storage/ipc.h index 7b4eb7734e2..cb9ed1ffa13 100644 --- a/src/include/storage/ipc.h +++ b/src/include/storage/ipc.h @@ -11,21 +11,63 @@ * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/storage/ipc.h,v 1.72 2006/03/05 15:58:59 momjian Exp $ + * $PostgreSQL: pgsql/src/include/storage/ipc.h,v 1.72.2.1 2008/04/17 00:00:01 tgl Exp $ * *------------------------------------------------------------------------- */ #ifndef IPC_H #define IPC_H +typedef void (*pg_on_exit_callback) (int code, Datum arg); + +/*---------- + * API for handling cleanup that must occur during either ereport(ERROR) + * or ereport(FATAL) exits from a block of code. (Typical examples are + * undoing transient changes to shared-memory state.) + * + * PG_ENSURE_ERROR_CLEANUP(cleanup_function, arg); + * { + * ... code that might throw ereport(ERROR) or ereport(FATAL) ... + * } + * PG_END_ENSURE_ERROR_CLEANUP(cleanup_function, arg); + * + * where the cleanup code is in a function declared per pg_on_exit_callback. + * The Datum value "arg" can carry any information the cleanup function + * needs. + * + * This construct ensures that cleanup_function() will be called during + * either ERROR or FATAL exits. It will not be called on successful + * exit from the controlled code. (If you want it to happen then too, + * call the function yourself from just after the construct.) + * + * Note: the macro arguments are multiply evaluated, so avoid side-effects. + *---------- + */ +#define PG_ENSURE_ERROR_CLEANUP(cleanup_function, arg) \ + do { \ + on_shmem_exit(cleanup_function, arg); \ + PG_TRY() + +#define PG_END_ENSURE_ERROR_CLEANUP(cleanup_function, arg) \ + cancel_shmem_exit(cleanup_function, arg); \ + PG_CATCH(); \ + { \ + cancel_shmem_exit(cleanup_function, arg); \ + cleanup_function (0, arg); \ + PG_RE_THROW(); \ + } \ + PG_END_TRY(); \ + } while (0) + /* ipc.c */ extern bool proc_exit_inprogress; extern void proc_exit(int code); extern void shmem_exit(int code); -extern void on_proc_exit(void (*function) (int code, Datum arg), Datum arg); -extern void on_shmem_exit(void (*function) (int code, Datum arg), Datum arg); +extern void on_proc_exit(pg_on_exit_callback function, Datum arg); +extern void on_shmem_exit(pg_on_exit_callback function, Datum arg); +extern void cancel_shmem_exit(pg_on_exit_callback function, Datum arg); extern void on_exit_reset(void); /* ipci.c */ diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h index 1cbc734d271..707bc5b2fda 100644 --- a/src/include/utils/elog.h +++ b/src/include/utils/elog.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/utils/elog.h,v 1.82 2006/03/05 15:59:07 momjian Exp $ + * $PostgreSQL: pgsql/src/include/utils/elog.h,v 1.82.2.1 2008/04/17 00:00:01 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -196,6 +196,13 @@ extern DLLIMPORT ErrorContextCallback *error_context_stack; * of levels this will work for. It's best to keep the error recovery * section simple enough that it can't generate any new errors, at least * not before popping the error stack. + * + * Note: an ereport(FATAL) will not be caught by this construct; control will + * exit straight through proc_exit(). Therefore, do NOT put any cleanup + * of non-process-local resources into the error recovery section, at least + * not without taking thought for what will happen during ereport(FATAL). + * The PG_ENSURE_ERROR_CLEANUP macros provided by storage/ipc.h may be + * helpful in such cases. *---------- */ #define PG_TRY() \