I've attached an updated patch that fixes a failure when compiling on
gcc/linux. The no-op inline installCrashDumpHandler() for unsupported
platforms was not declared static, so it was not being optimized out of
objects it wasn't used in and was causing symbol collisions during linkage.
"make check" now passes correctly on Linux and no warnings are generated
during compilation.
=======================
All 124 tests passed.
=======================
Sorry about this mess-up; I don't recall having to use "inline static"
when using inlines in headers with c++, but as that was a while ago (and
C++) my recollection can't be trusted anyway.
Your thoughts on the following questions from before would be much
appreciated:
> I'm not sure I've taken the
right approach in terms of how I've hooked the crash dump code into the
build system. I'm fairly happy with when it's actually inited, and with
the win32 portfile (ports/win32/crashdump.c) - what I'm not so sure
about is how I've handled the conditional compilation by platform.
Currently, src/backend/ports/win32/crashdump.c is compiled on win32. It
exports a generic win32-garbage-free function:
void
installCrashDumpHandler();
I was just going to create a no-op version of the same function in a
(new) file src/backend/ports/crashdump.c , but I was having problems
figuring out how to sensibly express "compile this file for everything
except these ports". In addition, doing it this way means there's still
a pointless no-op function call in each backend start and some useless
symbols - maybe not a big worry, but not ideal.
What I ended up doing was putting a conditionally compiled switch in
port.h that produces an inline no-op version of installCrashDumpHandler
for unsupported platforms, and an extern for supported platforms
(currently win32). That'll optimize out completely on unsupported
platforms, which is nice if unimportant.
I'm just not sure if port.h is really the right place and if it's only
used by the backend. I considered a new header included by postgres.h
(since it's really backend-only stuff) but as it seems Pg generally
prefers bigger headers over more little headers, I popped it in port.h .
>From de9a1f0e0f4ccfd8b33f37212aea61fd6ba29a89 Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@ayaki.(none)>
Date: Tue, 14 Dec 2010 13:47:09 +0800
Subject: [PATCH] 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 2 of the patch, including a fix for a warning when
compiling on Linux/gcc.
---
src/backend/main/main.c | 7 ++
src/backend/port/win32/Makefile | 2 +-
src/backend/port/win32/crashdump.c | 202 ++++++++++++++++++++++++++++++++++++
src/include/port.h | 13 +++
src/include/postgres.h | 4 +
5 files changed, 227 insertions(+), 1 deletions(-)
create mode 100644 src/backend/port/win32/crashdump.c
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/win32/Makefile b/src/backend/port/win32/Makefile
index 8bf9f74..d00c334 100644
--- a/src/backend/port/win32/Makefile
+++ b/src/backend/port/win32/Makefile
@@ -12,6 +12,6 @@ subdir = src/backend/port/win32
top_builddir = ../../../..
include $(top_builddir)/src/Makefile.global
-OBJS = timer.o socket.o signal.o security.o mingwcompat.o
+OBJS = timer.o socket.o signal.o security.o mingwcompat.o crashdump.o
include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/port/win32/crashdump.c
b/src/backend/port/win32/crashdump.c
new file mode 100644
index 0000000..a5629ea
--- /dev/null
+++ b/src/backend/port/win32/crashdump.c
@@ -0,0 +1,202 @@
+/*-------------------------------------------------------------------------
+ *
+ * crashdump.c
+ * Automatic crash dump creation for PostgreSQL on Windows
+ *
+ * The crashdump module traps unhandled 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.
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#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()
+{
+ // 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()
+{
+ // http://msdn.microsoft.com/en-us/library/ms680634(VS.85).aspx
+ SetUnhandledExceptionFilter( crashDumpHandler );
+}
diff --git a/src/include/port.h b/src/include/port.h
index 0dbc322..a657755 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -447,4 +447,17 @@ extern int pg_check_dir(const char *dir);
/* port/pgmkdirp.c */
extern int pg_mkdir_p(char *path, int omode);
+#if defined(WIN32)
+ /* For platforms that have a crashdump handler in
src/port/$platform/crashdump.c,
+ * reference it. */
+ extern void
+ installCrashDumpHandler();
+#else
+ /* Provide a no-op inline function for platforms with no handler */
+ inline static void
+ installCrashDumpHandler()
+ {
+ }
+#endif
+
#endif /* PG_PORT_H */
diff --git a/src/include/postgres.h b/src/include/postgres.h
index b8b8037..7897bbb 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);
+
+/* Install a platform-specific crash dump handler routine from
port/crashdump.c */
+extern void installCrashDumpHandler();
+
#endif /* POSTGRES_H */
--
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
