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

Reply via email to