On Sun, Dec 19, 2010 at 12:51, Magnus Hagander <mag...@hagander.net> wrote:
> On Sun, Dec 19, 2010 at 07:26, Craig Ringer <cr...@postnewspapers.com.au> 
> wrote:
>> On 18/12/2010 1:13 AM, Magnus Hagander wrote:
>>>
>>> On Fri, Dec 17, 2010 at 17:42, Magnus Hagander<mag...@hagander.net>
>>>  wrote:
>>>>
>>>> On Fri, Dec 17, 2010 at 17:24, Craig Ringer<cr...@postnewspapers.com.au>
>>>>  wrote:
>>>>>
>>>>> On 17/12/2010 7:17 PM, Magnus Hagander wrote:
>>>>
>>>> Now, that's annoying. So clearly we can't use that function to
>>>> determine which version we're on. Seems it only works for "image help
>>>> api", and not the general thing.
>>>>
>>>> According to
>>>> http://msdn.microsoft.com/en-us/library/ms679294(v=vs.85).aspx,
>>>> we could look for:
>>>>
>>>> SysEnumLines - if present, we have at least 6.1.
>>>>
>>>> However, I don't see any function that appeared in 6.0 only..
>>>
>>> Actually, I'm wrong - there are functions enough to determine the
>>> version. So here's a patch that tries that.
>>
>> Great. I pulled the latest from your git tree, tested that, and got much
>> better results. Crashdump size is back to what I expected. In my test code,
>> fcinfo->args and fcinfo->argnull can be examined without problems.
>> Backtraces look good; see below. It seems to be including backend private
>> memory again now. Thanks _very_ much for your work on this.
>
> Ok, great. I think that leaves us at least complete enough to commit -
> we can always improve things further as we get some more real world
> testing.

Actually, looking through it again just before commit, I can't help
but think the whole code about loading the DLL from different places
is unnecessary. The windows DLL search order *always* has the
directory of the EXE first, followed by either system dirs or CWD
depending on version.

Any reason not to just call LoadLibrary once?

That makes the patch look like attached.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index 0eeaa25..a480a8d 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -2734,6 +2734,22 @@ cc-1020 cc: ERROR File = pqcomm.c, Line = 427
     under <command>CMD.EXE</command>, as the MSYS console has
     buffering issues.
    </para>
+
+   <sect3>
+    <title>Collecting crash dumps on Windows</title>
+
+    <para>
+     If PostgreSQL on Windows crashes, it has the ability to generate
+     <productname>minidumps</> that can be used to track down the cause
+     for the crash, similar to core dumps on Unix. These dumps can be
+     read using the <productname>Windows Debugger Tools</> or using
+     <productname>Visual Studio</>. To enable the generation of dumps
+     on Windows, create a subdirectory named <filename>crashdumps</filename>
+     inside the cluster data directory. The dumps will then be written
+     into this directory with a unique name based on the identifier of
+     the crashing process and the current time of the crash.
+    </para>
+   </sect3>
   </sect2>
 
   <sect2 id="installation-notes-sco">
diff --git a/src/backend/main/main.c b/src/backend/main/main.c
index 6cb70f2..5a56c25 100644
--- a/src/backend/main/main.c
+++ b/src/backend/main/main.c
@@ -79,6 +79,14 @@ 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.
+	 */
+#ifdef WIN32
+	pgwin32_install_crashdump_handler();
+#endif
+
+	/*
 	 * 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..ba6fca7
--- /dev/null
+++ b/src/backend/port/win32/crashdump.c
@@ -0,0 +1,174 @@
+/*-------------------------------------------------------------------------
+ *
+ * win32_crashdump.c
+ *         Automatic crash dump creation for PostgreSQL on Windows
+ *
+ * The crashdump feature traps unhandled win32 exceptions produced by the
+ * backend, 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 it self.
+ *
+ * POSSIBLE FUTURE WORK
+ * ====================
+ * For bonus points, the crash dump format permits embedding of user-supplied
+ * data. If there's anything else that should always be supplied with a crash
+ * dump (postgresql.conf? Last few lines of a log file?), it could potentially
+ * be added, though at the cost of a greater chance of the crash dump failing.
+ *
+ *
+ * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ *	  src/backend/port/win32/crashdump.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#define WIN32_LEAN_AND_MEAN
+#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
+ *
+ * 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
+									);
+
+
+/*
+ * 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 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 functions.
+ */
+static LONG WINAPI
+crashDumpHandler(struct _EXCEPTION_POINTERS *pExceptionInfo)
+{
+	/*
+	 * We only write crash dumps if the "crashdumps" directory within
+	 * the postgres data directory exists.
+	 */
+	DWORD attribs = GetFileAttributesA("crashdumps");
+	if (attribs != INVALID_FILE_ATTRIBUTES && (attribs & FILE_ATTRIBUTE_DIRECTORY) )
+	{
+		/* 'crashdumps' exists and is a directory. Try to write a dump' */
+		HMODULE hDll = NULL;
+		MINIDUMPWRITEDUMP pDump = NULL;
+		MINIDUMP_TYPE dumpType;
+		char dumpPath[_MAX_PATH];
+		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;
+
+		/* Load the dbghelp.dll library and functions */
+		hDll = LoadLibrary("dbghelp.dll");
+		if (hDll == NULL)
+		{
+			write_stderr("could not load dbghelp.dll, cannot write crashdump\n");
+			return EXCEPTION_CONTINUE_SEARCH;
+		}
+
+		pDump = (MINIDUMPWRITEDUMP)GetProcAddress(hDll, "MiniDumpWriteDump");
+
+		if (pDump==NULL)
+		{
+			write_stderr("could not load required functions in dbghelp.dll, cannot write crashdump\n");
+			return EXCEPTION_CONTINUE_SEARCH;
+		}
+
+		/*
+		 * Dump as much as we can, except shared memory, code segments,
+		 * and memory mapped files.
+		 * Exactly what we can dump depends on the version of dbghelp.dll,
+		 * see:
+		 * http://msdn.microsoft.com/en-us/library/ms680519(v=VS.85).aspx
+		 */
+		dumpType = MiniDumpNormal | MiniDumpWithHandleData |
+			MiniDumpWithDataSegs;
+
+		if (GetProcAddress(hDll, "EnumDirTree") != NULL)
+		{
+			/* If this function exists, we have version 5.2 or newer */
+			dumpType |= MiniDumpWithIndirectlyReferencedMemory |
+				MiniDumpWithPrivateReadWriteMemory;
+		}
+		if (GetProcAddress(hDll, "SymFromIndex") != NULL)
+		{
+			/* If this function exists, we have version 6.2 or newer */
+			dumpType |= MiniDumpWithThreadInfo;
+		}
+
+		systemTicks = GetTickCount();
+		snprintf(dumpPath, _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)
+		{
+			write_stderr("could not open crash dump file %s for writing: error code %d\n",
+					dumpPath, GetLastError());
+			return EXCEPTION_CONTINUE_SEARCH;
+		}
+
+		if ((*pDump)(selfProcHandle, selfPid, dumpFile, dumpType, &ExInfo,
+					 NULL, NULL))
+			write_stderr("wrote crash dump to %s\n", dumpPath);
+		else
+			write_stderr("could not write crash dump to %s: error code %08x\n",
+					dumpPath, GetLastError());
+
+		CloseHandle(dumpFile);
+	}
+
+	return EXCEPTION_CONTINUE_SEARCH;
+}
+
+
+void
+pgwin32_install_crashdump_handler(void)
+{
+	SetUnhandledExceptionFilter(crashDumpHandler);
+}
diff --git a/src/include/port/win32.h b/src/include/port/win32.h
index a9b9fca..93439f7 100644
--- a/src/include/port/win32.h
+++ b/src/include/port/win32.h
@@ -303,6 +303,9 @@ extern int	pgwin32_is_service(void);
 /* in backend/port/win32_shmem.c */
 extern int	pgwin32_ReserveSharedMemoryRegion(HANDLE);
 
+/* in backend/port/win32/crashdump.c */
+extern void pgwin32_install_crashdump_handler(void);
+
 /* in port/win32error.c */
 extern void _dosmaperr(unsigned long);
 
-- 
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