Hi all Updated patch attached. This one avoids the inline function stuff and instead just decides whether to compile unix_crashdump.c or win32_crashdump.c based on build system tests. It passes "make check" on nix and tests on win32, both with and without the "crashdumps" directory existing. The inadvertent duplication of the functon prototype is fixed.
I haven't added any SGML documentation yet, as I'm not sure (a) to what level it should be documented, and (b) whether it should be in the main docs or just the developer docs plus the crash debugging guide on the wiki. Thoughts? You can also find this patch at: https://github.com/ringerc/postgres/tree/crashdump ie git://github.com/ringerc/postgres.git, branch "crashdump" . -- System & Network Administrator POST Newspapers
>From b6b3052fab356971cf38a48d675c68b1767a7ad5 Mon Sep 17 00:00:00 2001 From: Craig Ringer <cr...@ayaki.(none)> Date: Thu, 16 Dec 2010 15:11:19 +0800 Subject: [PATCH] Add support for generating a crash dump on backend/postmaster crash On win32, services running under accounts without the right to interact with the desktop don't work well with the system's built-in crash dumping and error reporting facilities. For that reason, the crashdump code installs an unhandled exception handler to trap failures and have a crashing backend or postmaster debug its self. This handler loads and calls dbghelp.dll to generate a minidump when it is invoked. The minidump may be debugged after the backend has finished dying, or sent elsewhere for someone else to debug. Minidumps are only written if a directory called 'crashdumps' exists off the root of the datadir. If that directory exists, a backend crash will result in a minidump file being written to that directory, containing the pid of the crashing backend and a system tick count to prevent overwrites on pid wraparound. The filename of the dump is written to the postgresql log (if possible) before the backend resumes crashing. If the crashdumps directory does NOT exist, a log message is emitted from the crashing backend to indicate why no dump was written before it crashes. Minidumps may be debugged using windbg.exe or a paid version of Microsoft Visual Studio, though not with the free Visual Studio Express Edition. In future a helper process or even the postmaster could take responsibility for debugging a crashing process, so it doesn't need to try to debug its self. If the postmaster gains the ability to manage generic (non-worker-backend children) then a crash handler/reporter would be a useful lightweight option. This would also offer the opportunity to have log excerpts, config files, or other important resources automatically included in the crashdump. Crash dump support is handled via the ports infrastructure. Currently installation of a crash dump handler is a no-op on platforms other than win32, so this patch only meaningfully affects Windows systems. This infrastructure may theoretically be extended to support self-dumps on other platforms, though most UNIX platforms offer OS-level core dump features that remove the need for application software to worry about this. This is revision 4 of the patch. The static inline to no-op out the install call was replaced by a proper extern that calls an empty function, after comments by Tom Lane. An accidental duplication of the prototype in both port.h and postgres.h was fixed and the prototype was moved to postgres.h only. Trailing whitespace was fixed. Build integration was fixed to use the same mechanism as the rest of the ports infrastructure, and a new configure was generated to reflect the changes to configure.in. Squashed commit of the following: commit 4faef5aec63f3d3a6a01235b33f22ce0101c444d Author: Craig Ringer <cr...@ayaki.(none)> Date: Thu Dec 16 14:36:46 2010 +0800 Ignore autom4te.cache and the new pg_crashdump.c commit 9186496a2b444b67ab822ed45aea3a093269676e Author: Craig Ringer <cr...@ayaki.(none)> Date: Thu Dec 16 14:34:16 2010 +0800 New configure file to match configure.in changes for crashdump support As PostgreSQL versions the generated copy of `configure', re-generate it with autoconf-2.63 to reflect the changes to configure.in . The diff shows only changes related to the configure.in alterations, with no other random alterations made by configure. commit 2a2ad20a8bf4f4e3730b4e271b3956ebebfea87c Author: Craig Ringer <cr...@ayaki.(none)> Date: Thu Dec 16 14:02:24 2010 +0800 Correctly integrate crashdump support into the build system Use configure / Mkvcbuild to switch between win32_crashdump.c and unix_crashdump.c rather than trying to do it with Make conditionals. Document the right way to do this for the future reference of others. commit 19bd84f3ec459744fb9df1bc2b22522f031aee2d Author: Craig Ringer <cr...@ayaki.(none)> Date: Thu Dec 16 13:21:28 2010 +0800 Add support for generating a crash dump on backend/postmaster crash Crash dump support is handled via the ports infrastructure. Currently installation of a crash dump handler is a no-op on platforms other than win32. On win32, an unhandled exception handler is installed. This handler loads and calls dbghelp.dll to generate a minidump when it is invoked. The minidump may be debugged after the backend has finished dying, or sent elsewhere for someone else to debug. This is revision 3 of the patch. The static inline to no-op out the install call was replaced by a proper extern that calls an empty function, after comments by Tom Lane. An accidental duplication of the prototype in both port.h and postgres.h was fixed and the prototype was moved to postgres.h only. Trailing whitespace was fixed. --- .gitignore | 1 + configure | 10 ++- configure.in | 8 ++ src/backend/main/main.c | 7 ++ src/backend/port/.gitignore | 1 + src/backend/port/Makefile | 7 +- src/backend/port/unix_crashdump.c | 23 ++++ src/backend/port/win32_crashdump.c | 208 ++++++++++++++++++++++++++++++++++++ src/include/postgres.h | 4 + src/tools/msvc/Mkvcbuild.pm | 1 + 10 files changed, 268 insertions(+), 2 deletions(-) create mode 100644 src/backend/port/unix_crashdump.c create mode 100644 src/backend/port/win32_crashdump.c diff --git a/.gitignore b/.gitignore index 1be11e8..96138be 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,4 @@ objfiles.txt /GNUmakefile /config.log /config.status +/autom4te.cache/ diff --git a/configure b/configure index 51d27d8..064452f 100755 --- a/configure +++ b/configure @@ -27854,6 +27854,13 @@ else LATCH_IMPLEMENTATION="src/backend/port/win32_latch.c" fi +# Select crashdump implementation type. +if test "$PORTNAME" != "win32"; then + CRASHDUMP_IMPLEMENTATION="src/backend/port/unix_crashdump.c" +else + CRASHDUMP_IMPLEMENTATION="src/backend/port/win32_crashdump.c" +fi + # If not set in template file, set bytes to use libc memset() if test x"$MEMSET_LOOP_LIMIT" = x"" ; then MEMSET_LOOP_LIMIT=1024 @@ -29194,7 +29201,7 @@ fi ac_config_files="$ac_config_files GNUmakefile src/Makefile.global" -ac_config_links="$ac_config_links src/backend/port/dynloader.c:src/backend/port/dynloader/${template}.c src/backend/port/pg_sema.c:${SEMA_IMPLEMENTATION} src/backend/port/pg_shmem.c:${SHMEM_IMPLEMENTATION} src/backend/port/pg_latch.c:${LATCH_IMPLEMENTATION} src/include/dynloader.h:src/backend/port/dynloader/${template}.h src/include/pg_config_os.h:src/include/port/${template}.h src/Makefile.port:src/makefiles/Makefile.${template}" +ac_config_links="$ac_config_links src/backend/port/dynloader.c:src/backend/port/dynloader/${template}.c src/backend/port/pg_sema.c:${SEMA_IMPLEMENTATION} src/backend/port/pg_shmem.c:${SHMEM_IMPLEMENTATION} src/backend/port/pg_latch.c:${LATCH_IMPLEMENTATION} src/backend/port/pg_crashdump.c:${CRASHDUMP_IMPLEMENTATION} src/include/dynloader.h:src/backend/port/dynloader/${template}.h src/include/pg_config_os.h:src/include/port/${template}.h src/Makefile.port:src/makefiles/Makefile.${template}" if test "$PORTNAME" = "win32"; then @@ -29819,6 +29826,7 @@ do "src/backend/port/pg_sema.c") CONFIG_LINKS="$CONFIG_LINKS src/backend/port/pg_sema.c:${SEMA_IMPLEMENTATION}" ;; "src/backend/port/pg_shmem.c") CONFIG_LINKS="$CONFIG_LINKS src/backend/port/pg_shmem.c:${SHMEM_IMPLEMENTATION}" ;; "src/backend/port/pg_latch.c") CONFIG_LINKS="$CONFIG_LINKS src/backend/port/pg_latch.c:${LATCH_IMPLEMENTATION}" ;; + "src/backend/port/pg_crashdump.c") CONFIG_LINKS="$CONFIG_LINKS src/backend/port/pg_crashdump.c:${CRASHDUMP_IMPLEMENTATION}" ;; "src/include/dynloader.h") CONFIG_LINKS="$CONFIG_LINKS src/include/dynloader.h:src/backend/port/dynloader/${template}.h" ;; "src/include/pg_config_os.h") CONFIG_LINKS="$CONFIG_LINKS src/include/pg_config_os.h:src/include/port/${template}.h" ;; "src/Makefile.port") CONFIG_LINKS="$CONFIG_LINKS src/Makefile.port:src/makefiles/Makefile.${template}" ;; diff --git a/configure.in b/configure.in index b999b94..1c65dbc 100644 --- a/configure.in +++ b/configure.in @@ -1716,6 +1716,13 @@ else LATCH_IMPLEMENTATION="src/backend/port/win32_latch.c" fi +# Select crashdump implementation type. +if test "$PORTNAME" != "win32"; then + CRASHDUMP_IMPLEMENTATION="src/backend/port/unix_crashdump.c" +else + CRASHDUMP_IMPLEMENTATION="src/backend/port/win32_crashdump.c" +fi + # If not set in template file, set bytes to use libc memset() if test x"$MEMSET_LOOP_LIMIT" = x"" ; then MEMSET_LOOP_LIMIT=1024 @@ -1858,6 +1865,7 @@ AC_CONFIG_LINKS([ src/backend/port/pg_sema.c:${SEMA_IMPLEMENTATION} src/backend/port/pg_shmem.c:${SHMEM_IMPLEMENTATION} src/backend/port/pg_latch.c:${LATCH_IMPLEMENTATION} + src/backend/port/pg_crashdump.c:${CRASHDUMP_IMPLEMENTATION} src/include/dynloader.h:src/backend/port/dynloader/${template}.h src/include/pg_config_os.h:src/include/port/${template}.h src/Makefile.port:src/makefiles/Makefile.${template} diff --git a/src/backend/main/main.c b/src/backend/main/main.c index 6cb70f2..45a25f4 100644 --- a/src/backend/main/main.c +++ b/src/backend/main/main.c @@ -79,6 +79,13 @@ main(int argc, char *argv[]) argv = save_ps_display_args(argc, argv); /* + * If supported on the current platform, set up a handler to be called if + * the backend/postmaster crashes with a fatal signal or exception. + * See port/crashdump.c + */ + installCrashDumpHandler(); + + /* * Set up locale information from environment. Note that LC_CTYPE and * LC_COLLATE will be overridden later from pg_control if we are in an * already-initialized database. We set them here so that they will be diff --git a/src/backend/port/.gitignore b/src/backend/port/.gitignore index 7d3ac4a..e69547e 100644 --- a/src/backend/port/.gitignore +++ b/src/backend/port/.gitignore @@ -3,3 +3,4 @@ /pg_sema.c /pg_shmem.c /tas.s +/pg_crashdump.c diff --git a/src/backend/port/Makefile b/src/backend/port/Makefile index 8ebb6d5..acd58eb 100644 --- a/src/backend/port/Makefile +++ b/src/backend/port/Makefile @@ -21,7 +21,12 @@ subdir = src/backend/port top_builddir = ../../.. include $(top_builddir)/src/Makefile.global -OBJS = dynloader.o pg_sema.o pg_shmem.o pg_latch.o $(TAS) +# The pg_ files are selected from several alternative implementations +# by code in configure.in and src/tools/msvc/Mkvcbuild.pm (for win32). +# Use that mechanism rather than trying to use Make conditionals, because +# the win32 perl-based build scripts don't understand Make conditionals +# very well. +OBJS = dynloader.o pg_sema.o pg_shmem.o pg_latch.o pg_crashdump.o $(TAS) ifeq ($(PORTNAME), darwin) SUBDIRS += darwin diff --git a/src/backend/port/unix_crashdump.c b/src/backend/port/unix_crashdump.c new file mode 100644 index 0000000..f209293 --- /dev/null +++ b/src/backend/port/unix_crashdump.c @@ -0,0 +1,23 @@ +/*------------------------------------------------------------------------- + * + * unix_crashdump.c + * No-op crash dump handler for unsupported platforms + * + * No support for generating crash dumps has been implemented for unix + * platforms, but an implementation of "void installCrashDumpHandler(void)" + * is still required. + * + * Copyright (c) 1996-2010, PostgreSQL Global Development Group + * + * IDENTIFICATION + * src/backend/port/unix_crashdump.c + * + *------------------------------------------------------------------------- + */ + +#include "postgres.h" + +void +installCrashDumpHandler(void) +{ +} diff --git a/src/backend/port/win32_crashdump.c b/src/backend/port/win32_crashdump.c new file mode 100644 index 0000000..f07917b --- /dev/null +++ b/src/backend/port/win32_crashdump.c @@ -0,0 +1,208 @@ +/*------------------------------------------------------------------------- + * + * win32_crashdump.c + * Automatic crash dump creation for PostgreSQL on Windows + * + * The crashdump feature traps unhandled win32 exceptions produced by the backend + * the module is loaded in, and tries to produce a Windows MiniDump crash + * dump for later debugging and analysis. The machine performing the dump + * doesn't need any special debugging tools; the user only needs to send + * the dump to somebody who has the same version of PostgreSQL and has debugging + * tools. + * + * crashdump module originally by Craig Ringer <ring...@ringerc.id.au> + * + * LIMITATIONS: + * ============ + * This *won't* work in hard OOM situations or stack overflows. + * + * For those, it'd be necessary to take a much more complicated approach where + * the handler switches to a new stack (if it can) and forks a helper process + * to debug its self. That's in the too hard basket as far as I'm concerned; + * this approach will get 90% of the results with 10% of the work. + * + * POSSIBLE FUTURE WORK: + * ===================== + * For bonus points, the crash dump format permits embedding of user-supplied data. + * If there's anything else (postgresql.conf? Last few lines of a log file?) that + * should always be supplied with a crash dump, it could potentially be added, + * though at the cost of a greater chance of the crash dump failing. Again, + * I'm not going to tackle that, but thought it worth mentioning in case + * someone wants it down the track. + * + * + * Copyright (c) 1996-2010, PostgreSQL Global Development Group + * + * IDENTIFICATION + * src/backend/port/win32_crashdump.c + * + *------------------------------------------------------------------------- + */ + +#include "postgres.h" + +#define VC_EXTRALEAN +#include <windows.h> +#include <string.h> +#include <dbghelp.h> + +/* + * Much of the following code is based on CodeProject and MSDN examples, + * particularly http://www.codeproject.com/KB/debug/postmortemdebug_standalone1.aspx + * + * Useful MSDN articles: + * + * http://msdn.microsoft.com/en-us/library/ff805116(v=VS.85).aspx + * http://msdn.microsoft.com/en-us/library/ms679294(VS.85).aspx + * http://msdn.microsoft.com/en-us/library/ms679291(VS.85).aspx + * http://msdn.microsoft.com/en-us/library/ms680519(v=VS.85).aspx + * http://msdn.microsoft.com/en-us/library/ms680360(VS.85).aspx + * + * Other useful articles on working with minidumps: + * http://www.debuginfo.com/articles/effminidumps.html + */ + +typedef BOOL (WINAPI *MINIDUMPWRITEDUMP)(HANDLE hProcess, DWORD dwPid, HANDLE hFile, MINIDUMP_TYPE DumpType, + CONST PMINIDUMP_EXCEPTION_INFORMATION ExceptionParam, + CONST PMINIDUMP_USER_STREAM_INFORMATION UserStreamParam, + CONST PMINIDUMP_CALLBACK_INFORMATION CallbackParam + ); + +/* + * To perform a crash dump, we need to load dbghelp.dll and find the + * MiniDumpWriteDump function. If possible, the copy of dbghelp.dll + * shipped with PostgreSQL is loaded; failing that, the system copy will + * be used. + * + * This function is called in crash handler context. It can't trust that much + * of the backend is working. + * + * If NULL is returned, loading failed and the crash dump handler should + * not try to continue. + */ +static MINIDUMPWRITEDUMP +loadDbgHelp(void) +{ + // Use the dbghelp.dll shipped with Pg if we can find and load + // it, otherwise fall back to the copy installed in Windows. + HMODULE hDll = NULL; + MINIDUMPWRITEDUMP pDump = NULL; + char dbgHelpPath[_MAX_PATH]; + if (GetModuleFileName( NULL, dbgHelpPath, _MAX_PATH )) + { + char *slash = strrchr( dbgHelpPath, '\\' ); + if (slash) + { + strcpy( slash+1, "DBGHELP.DLL" ); + hDll = LoadLibrary( dbgHelpPath ); + } + } + if (hDll==NULL) + { + /* + * Load whichever version of dbghelp.dll we can find. + * + * It is safe to call LoadLibrary with an unqualified name (thus searching + * the PATH) only because the postmaster ensures that backends run in a sensible + * environment with the datadir as the working directory. It's usually unsafe to + * LoadLibrary("unqualifiedname.dll") because the user can run your program with + * a CWD containing a malicious copy of "unqualifiedname.dll" thanks to the way + * windows (rather insecurely) includes the CWD in the PATH by default. + */ + hDll = LoadLibrary( "DBGHELP.DLL" ); + } + if (hDll!=NULL) + { + pDump = (MINIDUMPWRITEDUMP)GetProcAddress( hDll, "MiniDumpWriteDump" ); + } + return pDump; +} + +/* + * This function is the exception handler passed to SetUnhandledExceptionFilter. It's invoked + * only if there's an unhandled exception. The handler will use dbghelp.dll to generate a crash + * dump, then resume the normal unhandled exception process, which will generally exit with a + * an error message from the runtime. + * + * This function is run under the unhandled exception handler, effectively + * in a crash context, so it should be careful with memory and avoid using + * any PostgreSQL API or other functions that use PostgreSQL API. For that reason, + * logging calls should not be made until after the dump has been written + * successfully. + * + */ +static LONG WINAPI +crashDumpHandler(struct _EXCEPTION_POINTERS *pExceptionInfo) +{ + /* We only write crash dumps if the 'crashdumps' directory within + * the postgres data directory exists. That's the documented way to + * turn crash dumps on and off. */ + DWORD attribs = GetFileAttributesA("crashdumps"); + if (attribs != INVALID_FILE_ATTRIBUTES && (attribs & FILE_ATTRIBUTE_DIRECTORY) ) + { + /* 'crashdumps' exists and is a directory. Try to write a dump' */ + MINIDUMPWRITEDUMP pDump = NULL; + char dumpPath[_MAX_PATH]; + + // Dump pretty much everything except shared memory, code segments, and memory mapped files + MINIDUMP_TYPE dumpType = MiniDumpNormal|\ + MiniDumpWithIndirectlyReferencedMemory|\ + MiniDumpWithHandleData|\ + MiniDumpWithThreadInfo|\ + MiniDumpWithPrivateReadWriteMemory| + MiniDumpWithDataSegs; + + HANDLE selfProcHandle = GetCurrentProcess(); + DWORD selfPid = GetProcessId(selfProcHandle); + HANDLE dumpFile; + DWORD systemTicks; + + struct _MINIDUMP_EXCEPTION_INFORMATION ExInfo; + ExInfo.ThreadId = GetCurrentThreadId(); + ExInfo.ExceptionPointers = pExceptionInfo; + ExInfo.ClientPointers = FALSE; + + pDump = loadDbgHelp(); + if (pDump==NULL) + { + elog(WARNING, "crashdump: Cannot write dump, failed to load dbghelp.dll"); + return EXCEPTION_CONTINUE_SEARCH; + } + + systemTicks = GetTickCount(); + snprintf(&dumpPath[0], _MAX_PATH, "crashdumps\\postgres-pid%0i-%0i.mdmp", selfPid, systemTicks); + dumpPath[_MAX_PATH-1] = '\0'; + dumpFile = CreateFile( dumpPath, GENERIC_WRITE, FILE_SHARE_WRITE,\ + NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL ); + if (dumpFile==INVALID_HANDLE_VALUE) + { + elog(WARNING, "crashdump: Unable to open dump file %s for writing (win32 error %i)", + &dumpPath[0], GetLastError()); + elog(DEBUG1, "crashdump: is there a 'crashdump' directory within the data dir, and is it writable by the postgres user?"); + return EXCEPTION_CONTINUE_SEARCH; + } + + if( (*pDump)( selfProcHandle, selfPid, dumpFile, dumpType, &ExInfo, NULL, NULL ) ) + { + elog(WARNING,"crashdump: wrote crash dump to %s", &dumpPath[0]); + } + else + { + elog(WARNING,"crashdump: failed to write dump file to %s (win32 error %i)", + &dumpPath[0], GetLastError()); + } + CloseHandle(dumpFile); + } + else + { + elog(WARNING, "crashdump: Not writing dump, output directory 'crashdumps' missing or not writable."); + } + return EXCEPTION_CONTINUE_SEARCH; +} + +void +installCrashDumpHandler(void) +{ + // http://msdn.microsoft.com/en-us/library/ms680634(VS.85).aspx + SetUnhandledExceptionFilter( crashDumpHandler ); +} diff --git a/src/include/postgres.h b/src/include/postgres.h index b8b8037..3d13772 100644 --- a/src/include/postgres.h +++ b/src/include/postgres.h @@ -687,4 +687,8 @@ extern int ExceptionalCondition(const char *conditionName, const char *errorType, const char *fileName, int lineNumber); +/* backend/port/[unix|win32]_crashdump.c */ +extern void +installCrashDumpHandler(); + #endif /* POSTGRES_H */ diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm index d24543b..b950fdd 100644 --- a/src/tools/msvc/Mkvcbuild.pm +++ b/src/tools/msvc/Mkvcbuild.pm @@ -66,6 +66,7 @@ sub mkvcbuild $postgres->ReplaceFile('src\backend\port\pg_sema.c','src\backend\port\win32_sema.c'); $postgres->ReplaceFile('src\backend\port\pg_shmem.c','src\backend\port\win32_shmem.c'); $postgres->ReplaceFile('src\backend\port\pg_latch.c','src\backend\port\win32_latch.c'); + $postgres->ReplaceFile('src\backend\port\pg_crashdump.c','src\backend\port\win32_crashdump.c'); $postgres->AddFiles('src\port',@pgportfiles); $postgres->AddDir('src\timezone'); $postgres->AddFiles('src\backend\parser','scan.l','gram.y'); -- 1.7.3.1
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers