pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedWindows crash / abort handling
12+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2022-02-03 f3feff8259 windows: Improve crash / assert / exception handling.
2022-02-03 6dcc185266 ci: windows: run tests under timeout.
* Windows crash / abort handling
@ 2021-10-05 19:30 Andres Freund <andres@anarazel.de>
0 siblings, 2 replies; 12+ messages in thread
From: Andres Freund @ 2021-10-05 19:30 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Andrew Dunstan <andrew@dunslane.net>; Tom Lane <tgl@sss.pgh.pa.us>; Craig Ringer <craig.ringer@enterprisedb.com>
Hi,
As threatened in [1]... For CI, originally in the AIO project but now more
generally, I wanted to get windows backtraces as part of CI. I also was
confused why visual studio's "just in time debugging" (i.e. a window popping
up offering to debug a process when it crashes) didn't work with postgres.
My first attempt was to try to use the existing crashdump stuff in
pgwin32_install_crashdump_handler(). That's not really quite what I want,
because it only handles postmaster rather than any binary, but I thought it'd
be a good start. But outside of toy situations it didn't work for me.
A bunch of debugging later I figured out that the reason neither the
SetUnhandledExceptionFilter() nor JIT debugging works is that the
SEM_NOGPFAULTERRORBOX in the
SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX);
we do in startup_hacks() prevents the paths dealing with crashes from being
reached.
The SEM_NOGPFAULTERRORBOX hails from:
commit 27bff7502f04ee01237ed3f5a997748ae43d3a81
Author: Bruce Momjian <bruce@momjian.us>
Date: 2006-06-12 16:17:20 +0000
Prevent Win32 from displaying a popup box on backend crash. Instead let
the postmaster deal with it.
Magnus Hagander
I actually see error popups despite SEM_NOGPFAULTERRORBOX, at least for paths
reaching abort() (and thus our assertions).
The reason for abort() error boxes not being suppressed appears to be that in
debug mode a separate facility is reponsible for that: [2], [3]
"The default behavior is to print the message. _CALL_REPORTFAULT, if set,
specifies that a Watson crash dump is generated and reported when abort is
called. By default, crash dump reporting is enabled in non-DEBUG builds."
We apparently need _set_abort_behavior(_CALL_REPORTFAULT) to have abort()
behave the same between debug and release builds. [4]
To prevent the error popups we appear to at least need to call
_CrtSetReportMode(). The docs say:
If you do not call _CrtSetReportMode to define the output destination of
messages, then the following defaults are in effect:
Assertion failures and errors are directed to a debug message window.
We can configure it so that that stuff goes to stderr, by calling
_CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_FILE | _CRTDBG_MODE_DEBUG);
_CrtSetReportFile(_CRT_ASSERT, _CRTDBG_FILE_STDERR);
(and the same for _CRT_ERROR and perhaps _CRT_WARNING)
which removes the default _CRTDBG_MODE_WNDW.
It's possible that we'd need to do more than this, but this was sufficient to
get crash reports for segfaults and abort() in both assert and release builds,
without seeing an error popup.
To actually get the crash reports I ended up doing the following on the OS
level [5]:
Set-ItemProperty -Path 'HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\AeDebug' -Name 'Debugger' -Value '\"C:\Windows Kits\10\Debuggers\x64\cdb.exe\" -p %ld -e %ld -g -kqm -c \".lines -e; .symfix+ ;.logappend c:\cirrus\crashlog.txt ; !peb; ~*kP ; .logclose ; q \"' ; `
New-ItemProperty -Path 'HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\AeDebug' -Name 'Auto' -Value 1 -PropertyType DWord ; `
Get-ItemProperty -Path 'HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\AeDebug' -Name Debugger; `
This requires 'cdb' to be present, which is included in the Windows 10 SDK (or
other OS versions, it doesn't appear to have changed much). Whenever there's
an unhandled crash, cdb.exe is invoked with the parameters above, which
appends the crash report to crashlog.txt.
Alternatively we can generate "minidumps" [6], but that doesn't appear to be more
helpful for CI purposes at least - all we'd do is to create a backtrace using
the same tool. But it might be helpful for local development, to e.g. analyze
crashes in more detail.
The above ends up dumping all crashes into a single file, but that can
probably be improved. But cdb is so gnarly that I wanted to stop looking once
I got this far...
Andrew, I wonder if something like this could make sense for windows BF animals?
Greetings,
Andres Freund
[1] https://postgr.es/m/20211001222752.wrz7erzh4cajvgp6%40alap3.anarazel.de
[2] https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/crtsetreportmode?view=msvc-160
[3] https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/set-abort-behavior?view=msvc-160
[4] If anybody can explain to me what the two different parameters to
_set_abort_behavior() do, I'd be all ears
[5] https://docs.microsoft.com/en-us/windows/win32/debug/configuring-automatic-debugging
[6] https://docs.microsoft.com/en-us/windows/win32/wer/wer-settings
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Windows crash / abort handling
@ 2021-10-06 06:11 Craig Ringer <craig.ringer@enterprisedb.com>
parent: Andres Freund <andres@anarazel.de>
1 sibling, 1 reply; 12+ messages in thread
From: Craig Ringer @ 2021-10-06 06:11 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers; Andrew Dunstan <andrew@dunslane.net>; Tom Lane <tgl@sss.pgh.pa.us>
On Wed, 6 Oct 2021, 03:30 Andres Freund, <andres@anarazel.de> wrote:
> Hi,
>
>
> My first attempt was to try to use the existing crashdump stuff in
> pgwin32_install_crashdump_handler(). That's not really quite what I want,
> because it only handles postmaster rather than any binary, but I thought
> it'd
> be a good start. But outside of toy situations it didn't work for me.
>
Odd. It usually has for me, and definitely not limited to the postmaster.
But it will fall down for OOM, smashed stack, and other states where
in-process self-debugging is likely to fail.
>
> A bunch of debugging later I figured out that the reason neither the
> SetUnhandledExceptionFilter() nor JIT debugging works is that the
> SEM_NOGPFAULTERRORBOX in the
> SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX);
> we do in startup_hacks() prevents the paths dealing with crashes from being
> reached.
>
Right.
I patch this out when working on windows because it's a real pain.
I keep meaning to propose that we remove this functionality entirely. It's
obsolete. It was introduced back in the days where DrWatson.exe "windows
error reporting") used to launch an interactive prompt asking the user what
to do when a process crashed. This would block the crashed process from
exiting, making everything grind to a halt until the user interacted with
the
UI. Even for a service process.
Not fun on a headless or remote server.
These days Windows handles all this a lot more sensibly, and blocking crash
reporting is quite obsolete and unhelpful.
I'd like to just remove it.
If we can't do that I'd like to at least make it optional.
Alternatively we can generate "minidumps" [6], but that doesn't appear to
> be more
> helpful for CI purposes at least - all we'd do is to create a backtrace
> using
> the same tool. But it might be helpful for local development, to e.g.
> analyze
> crashes in more detail.
>
They're immensely helpful when a bt isn't enough, but BTs are certainly the
first step for CI use.
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Windows crash / abort handling
@ 2022-01-10 00:57 Andres Freund <andres@anarazel.de>
parent: Craig Ringer <craig.ringer@enterprisedb.com>
0 siblings, 1 reply; 12+ messages in thread
From: Andres Freund @ 2022-01-10 00:57 UTC (permalink / raw)
To: Craig Ringer <craig.ringer@enterprisedb.com>; +Cc: pgsql-hackers; Andrew Dunstan <andrew@dunslane.net>; Tom Lane <tgl@sss.pgh.pa.us>
Hi,
On 2021-10-06 14:11:51 +0800, Craig Ringer wrote:
> On Wed, 6 Oct 2021, 03:30 Andres Freund, <andres@anarazel.de> wrote:
>
> > Hi,
> >
> >
> > My first attempt was to try to use the existing crashdump stuff in
> > pgwin32_install_crashdump_handler(). That's not really quite what I want,
> > because it only handles postmaster rather than any binary, but I thought
> > it'd
> > be a good start. But outside of toy situations it didn't work for me.
> >
>
> Odd. It usually has for me, and definitely not limited to the postmaster.
> But it will fall down for OOM, smashed stack, and other states where
> in-process self-debugging is likely to fail.
I think it's a question of running debug vs optimized builds. At least in
debug builds it doesn't appear to work because the debug c runtime abort
preempts it.
> I patch this out when working on windows because it's a real pain.
>
> I keep meaning to propose that we remove this functionality entirely. It's
> obsolete. It was introduced back in the days where DrWatson.exe "windows
> error reporting") used to launch an interactive prompt asking the user what
> to do when a process crashed. This would block the crashed process from
> exiting, making everything grind to a halt until the user interacted with
> the
> UI. Even for a service process.
> Not fun on a headless or remote server.
Yea, the way we do it right now is definitely not helpful. Especially because
it doesn't actually prevent the "hang" issue - the CRT boxes at least cause
precisely such stalls.
We've had a few CI hangs due to such errors.
> These days Windows handles all this a lot more sensibly, and blocking crash
> reporting is quite obsolete and unhelpful.
From what I've seen it didn't actually get particularly sensible, just
different and more complicated.
From what I've seen one needs at least:
- _set_abort_behavior(_CALL_REPORTFAULT | _WRITE_ABORT_MSG)
- _set_error_mode(_OUT_TO_STDERR)
- _CrtSetReportMode(_CRT_ASSERT/ERROR, _CRTDBG_MODE_FILE | _CRTDBG_MODE_DEBUG)
- SetErrorMode(SEM_FAILCRITICALERRORS)
There's many things this hocuspocus can be called, but sensible isn't among my
word choices for it.
> I'd like to just remove it.
I think we need to remove the SEM_NOGPFAULTERRORBOX, but I don't think we can
remove the SEM_FAILCRITICALERRORS, and I think we need the rest of the above
to prevent error boxes from happening.
I think we ought to actually apply these to all binaries, not just
postgres. One CI hung was due to psql asserting. But there's currently no easy
hook point for all binaries afaict. If we were to introduce something it
should probably be in pgport? But I'd defer to that to a later step.
I've attached a patch implementing these changes.
I also made one run with that intentionally fail (with an Assert(false)), and
with the changes the debugger is invoked and creates a backtrace etc:
https://cirrus-ci.com/task/5447300246929408
(click on crashlog-> at the top)
Greetings,
Andres Freund
Attachments:
v6-0001-windows-Improve-crash-assert-exception-handling.patch (text/x-diff, 3.1K) download
From 4979447881836c86f0bf24a164d3ca920323f9eb Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Thu, 9 Sep 2021 17:49:39 -0700
Subject: [PATCH v6 1/2] windows: Improve crash / assert / exception handling.
---
src/backend/main/main.c | 48 +++++++++++++++++++++++++++++++++++++++--
.cirrus.yml | 6 +++++-
2 files changed, 51 insertions(+), 3 deletions(-)
diff --git a/src/backend/main/main.c b/src/backend/main/main.c
index 9124060bde7..111e7867cc7 100644
--- a/src/backend/main/main.c
+++ b/src/backend/main/main.c
@@ -22,6 +22,10 @@
#include <unistd.h>
+#if defined(WIN32)
+#include <crtdbg.h>
+#endif
+
#if defined(__NetBSD__)
#include <sys/param.h>
#endif
@@ -237,8 +241,48 @@ startup_hacks(const char *progname)
exit(1);
}
- /* In case of general protection fault, don't show GUI popup box */
- SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX);
+ /*
+ * By default abort() only generates a crash-dump in *non* debug
+ * builds. As our Assert() / ExceptionalCondition() uses abort(),
+ * leaving the default in place would make debugging harder.
+ */
+#if !defined(__MINGW32__) && !defined(__MINGW64__)
+ _set_abort_behavior(_CALL_REPORTFAULT | _WRITE_ABORT_MSG,
+ _CALL_REPORTFAULT | _WRITE_ABORT_MSG);
+#endif /* MINGW */
+
+ /*
+ * SEM_FAILCRITICALERRORS causes more errors to be reported to
+ * callers.
+ *
+ * We used to also specify SEM_NOGPFAULTERRORBOX, but that prevents
+ * windows crash reporting from working. Which includes registered
+ * just-in-time debuggers. Now we try to disable sources of popups
+ * separately below (note that SEM_NOGPFAULTERRORBOX didn't actually
+ * prevent all sources of such popups).
+ */
+ SetErrorMode(SEM_FAILCRITICALERRORS);
+
+ /*
+ * Show errors on stderr instead of popup box (note this doesn't
+ * affect errors originating in the C runtime, see below).
+ */
+ _set_error_mode(_OUT_TO_STDERR);
+
+ /*
+ * In DEBUG builds, errors, including assertions, C runtime errors are
+ * reported via _CrtDbgReport. By default such errors are displayed
+ * with a popup (even with NOGPFAULTERRORBOX), preventing forward
+ * progress. Instead report such errors stderr (and the
+ * debugger). This is C runtime specific and thus the above
+ * incantations aren't sufficient to suppress these popups.
+ */
+ _CrtSetReportMode(_CRT_ERROR, _CRTDBG_MODE_FILE | _CRTDBG_MODE_DEBUG);
+ _CrtSetReportFile(_CRT_ERROR, _CRTDBG_FILE_STDERR);
+ _CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_FILE | _CRTDBG_MODE_DEBUG);
+ _CrtSetReportFile(_CRT_ASSERT, _CRTDBG_FILE_STDERR);
+ _CrtSetReportMode(_CRT_WARN, _CRTDBG_MODE_FILE | _CRTDBG_MODE_DEBUG);
+ _CrtSetReportFile(_CRT_WARN, _CRTDBG_FILE_STDERR);
#if defined(_M_AMD64) && _MSC_VER == 1800
diff --git a/.cirrus.yml b/.cirrus.yml
index 19b3737fa11..89fbff14abb 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -423,7 +423,11 @@ task:
- cd src/tools/msvc
- perl vcregress.pl ecpgcheck
- on_failure: *on_failure
+ on_failure:
+ <<: *on_failure
+ crashlog_artifacts:
+ path: "crashlog-**.txt"
+ type: text/plain
task:
--
2.34.0
* Re: Windows crash / abort handling
@ 2022-01-10 15:57 Andrew Dunstan <andrew@dunslane.net>
parent: Andres Freund <andres@anarazel.de>
1 sibling, 1 reply; 12+ messages in thread
From: Andrew Dunstan @ 2022-01-10 15:57 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; pgsql-hackers; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Craig Ringer <craig.ringer@enterprisedb.com>
On 10/5/21 15:30, Andres Freund wrote
>
> To actually get the crash reports I ended up doing the following on the OS
> level [5]:
>
> Set-ItemProperty -Path 'HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\AeDebug' -Name 'Debugger' -Value '\"C:\Windows Kits\10\Debuggers\x64\cdb.exe\" -p %ld -e %ld -g -kqm -c \".lines -e; .symfix+ ;.logappend c:\cirrus\crashlog.txt ; !peb; ~*kP ; .logclose ; q \"' ; `
> New-ItemProperty -Path 'HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\AeDebug' -Name 'Auto' -Value 1 -PropertyType DWord ; `
> Get-ItemProperty -Path 'HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\AeDebug' -Name Debugger; `
>
> This requires 'cdb' to be present, which is included in the Windows 10 SDK (or
> other OS versions, it doesn't appear to have changed much). Whenever there's
> an unhandled crash, cdb.exe is invoked with the parameters above, which
> appends the crash report to crashlog.txt.
>
> Alternatively we can generate "minidumps" [6], but that doesn't appear to be more
> helpful for CI purposes at least - all we'd do is to create a backtrace using
> the same tool. But it might be helpful for local development, to e.g. analyze
> crashes in more detail.
>
> The above ends up dumping all crashes into a single file, but that can
> probably be improved. But cdb is so gnarly that I wanted to stop looking once
> I got this far...
>
>
> Andrew, I wonder if something like this could make sense for windows BF animals?
>
Very possibly. I wonder how well it will work on machines where I have
more than one animal .e.g. lorikeet (cygwin) jacana (msys) and bowerbird
(MSVC) are all on the same machine. Likewise drongo (MSVC) and fairywren
(msys2).
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Windows crash / abort handling
@ 2022-01-11 07:51 Andres Freund <andres@anarazel.de>
parent: Andrew Dunstan <andrew@dunslane.net>
0 siblings, 1 reply; 12+ messages in thread
From: Andres Freund @ 2022-01-11 07:51 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: pgsql-hackers@postgresql.org, Tom Lane <tgl@sss.pgh.pa.us>; Craig Ringer <craig.ringer@enterprisedb.com>
Hi,
On 2022-01-10 10:57:00 -0500, Andrew Dunstan wrote:
> On 10/5/21 15:30, Andres Freund wrote
> > The above ends up dumping all crashes into a single file, but that can
> > probably be improved. But cdb is so gnarly that I wanted to stop looking once
> > I got this far...
FWIW, I figured out how to put the dumps into separate files by now...
> > Andrew, I wonder if something like this could make sense for windows BF animals?
> Very possibly. I wonder how well it will work on machines where I have
> more than one animal .e.g. lorikeet (cygwin) jacana (msys) and bowerbird
> (MSVC) are all on the same machine. Likewise drongo (MSVC) and fairywren
> (msys2).
Hm. I can see a few ways to deal with it. Are they running concurrently?
If not then it's easy enough to deal with.
It'd be a bit of a fight with cdb's awfully documented and quirky
scripting [1], but the best solution would probably be to just use an
environment variable from the target process to determine the dump
location. Then each buildfarm config could set a BF_BACKTRACE_LOCATION
variable or such...
[1] So there's !envvar. But that yields a string like
BF_BACKTRACE_LOCATION = value of environment variable when set to an
alias. And I haven't found an easy way to get rid of the "variablename
= ". There is .foreach /pS [2] which could be used to skip over the
varname =, but that then splits on all whitespaces. Gah.
[2] https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/-foreach
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Windows crash / abort handling
@ 2022-01-11 17:01 Andrew Dunstan <andrew@dunslane.net>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 12+ messages in thread
From: Andrew Dunstan @ 2022-01-11 17:01 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers@postgresql.org, Tom Lane <tgl@sss.pgh.pa.us>; Craig Ringer <craig.ringer@enterprisedb.com>
On 1/11/22 02:51, Andres Freund wrote:
> Hi,
>
> On 2022-01-10 10:57:00 -0500, Andrew Dunstan wrote:
>> On 10/5/21 15:30, Andres Freund wrote
>>> The above ends up dumping all crashes into a single file, but that can
>>> probably be improved. But cdb is so gnarly that I wanted to stop looking once
>>> I got this far...
> FWIW, I figured out how to put the dumps into separate files by now...
>
>
>>> Andrew, I wonder if something like this could make sense for windows BF animals?
>> Very possibly. I wonder how well it will work on machines where I have
>> more than one animal .e.g. lorikeet (cygwin) jacana (msys) and bowerbird
>> (MSVC) are all on the same machine. Likewise drongo (MSVC) and fairywren
>> (msys2).
> Hm. I can see a few ways to deal with it. Are they running concurrently?
> If not then it's easy enough to deal with.
>
> It'd be a bit of a fight with cdb's awfully documented and quirky
> scripting [1], but the best solution would probably be to just use an
> environment variable from the target process to determine the dump
> location. Then each buildfarm config could set a BF_BACKTRACE_LOCATION
> variable or such...
>
> [1] So there's !envvar. But that yields a string like
> BF_BACKTRACE_LOCATION = value of environment variable when set to an
> alias. And I haven't found an easy way to get rid of the "variablename
> = ". There is .foreach /pS [2] which could be used to skip over the
> varname =, but that then splits on all whitespaces. Gah.
>
> [2] https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/-foreach
>
Ugly as heck. I normally don't use locations with spaces in them. Let's
assume we don't have to deal with that issue at least.
But in theory these animals could be running in parallel, and in theory
each animal could have more than one branch being run concurrently. In
fact locking them against each other can be difficult/impossible. From
experience, three different perls might not agree on how file locking
works ... In the case of fairywren/drongo I have had to set things up so
that there is a single batch file that runs one job after the other.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Windows crash / abort handling
@ 2022-01-11 21:13 Andres Freund <andres@anarazel.de>
parent: Andrew Dunstan <andrew@dunslane.net>
0 siblings, 1 reply; 12+ messages in thread
From: Andres Freund @ 2022-01-11 21:13 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: pgsql-hackers@postgresql.org, Tom Lane <tgl@sss.pgh.pa.us>; Craig Ringer <craig.ringer@enterprisedb.com>
Hi,
On 2022-01-11 12:01:42 -0500, Andrew Dunstan wrote:
> On 1/11/22 02:51, Andres Freund wrote:
> > It'd be a bit of a fight with cdb's awfully documented and quirky
> > scripting [1], but the best solution would probably be to just use an
> > environment variable from the target process to determine the dump
> > location. Then each buildfarm config could set a BF_BACKTRACE_LOCATION
> > variable or such...
> >
> > [1] So there's !envvar. But that yields a string like
> > BF_BACKTRACE_LOCATION = value of environment variable when set to an
> > alias. And I haven't found an easy way to get rid of the "variablename
> > = ". There is .foreach /pS [2] which could be used to skip over the
> > varname =, but that then splits on all whitespaces. Gah.
> >
> > [2] https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/-foreach
> >
>
> Ugly as heck.
Indeed. I think I figured it out:
0:000> !envvar frak
frak = C:\Program Files\Application Verifier\
0:000> ad /q path; .foreach /pS 2 (component {!envvar frak}){ .if (${/d:path}) {aS ${/v:path} ${/f:path} ${component}} .else {aS ${/v:path} ${component}}}; .block {.echo ${path}}
C:\Program Files\Application Verifier\
I mean, no explanation needed, right?
> But in theory these animals could be running in parallel, and in theory
> each animal could have more than one branch being run concurrently. In
> fact locking them against each other can be difficult/impossible.
The environment variable solution directing dumps for each animal / branch to
different directories should take care of that, right?
Do you have a preference where a script file implementing the necessary
cdb.exe commands would reside? It's getting too long to comfortably implement
it inside the registry setting itself... That script would be used by all
branches, rather than be branch specific.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Windows crash / abort handling
@ 2022-01-11 21:35 Andrew Dunstan <andrew@dunslane.net>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Dunstan @ 2022-01-11 21:35 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers@postgresql.org, Tom Lane <tgl@sss.pgh.pa.us>; Craig Ringer <craig.ringer@enterprisedb.com>
On 1/11/22 16:13, Andres Freund wrote:
> Hi,
>
> On 2022-01-11 12:01:42 -0500, Andrew Dunstan wrote:
>> On 1/11/22 02:51, Andres Freund wrote:
>>> It'd be a bit of a fight with cdb's awfully documented and quirky
>>> scripting [1], but the best solution would probably be to just use an
>>> environment variable from the target process to determine the dump
>>> location. Then each buildfarm config could set a BF_BACKTRACE_LOCATION
>>> variable or such...
>>>
>>> [1] So there's !envvar. But that yields a string like
>>> BF_BACKTRACE_LOCATION = value of environment variable when set to an
>>> alias. And I haven't found an easy way to get rid of the "variablename
>>> = ". There is .foreach /pS [2] which could be used to skip over the
>>> varname =, but that then splits on all whitespaces. Gah.
>>>
>>> [2] https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/-foreach
>>>
>> Ugly as heck.
> Indeed. I think I figured it out:
>
> 0:000> !envvar frak
> frak = C:\Program Files\Application Verifier\
> 0:000> ad /q path; .foreach /pS 2 (component {!envvar frak}){ .if (${/d:path}) {aS ${/v:path} ${/f:path} ${component}} .else {aS ${/v:path} ${component}}}; .block {.echo ${path}}
> C:\Program Files\Application Verifier\
>
> I mean, no explanation needed, right?
>
>
>> But in theory these animals could be running in parallel, and in theory
>> each animal could have more than one branch being run concurrently. In
>> fact locking them against each other can be difficult/impossible.
> The environment variable solution directing dumps for each animal / branch to
> different directories should take care of that, right?
>
>
> Do you have a preference where a script file implementing the necessary
> cdb.exe commands would reside? It's getting too long to comfortably implement
> it inside the registry setting itself... That script would be used by all
> branches, rather than be branch specific.
Well, the buildfarm code sets up a file for gdb in the branch's pgsql.build:
my $cmdfile = "./gdbcmd";
my $handle;
open($handle, '>', $cmdfile) || die "opening $cmdfile: $!";
print $handle "bt\n";
print $handle 'p $_siginfo', "\n";
close($handle);
my @trace = ("\n\n");
foreach my $core (@cores)
{
my @onetrace =
run_log("gdb -x $cmdfile --batch $bindir/postgres $core");
push(@trace,
"$log_file_marker stack trace: $core $log_file_marker\n",
@onetrace);
}
So by analogy we could do something similar for the Windows debugger. Or
we could pick up a file from the repo if we wanted to commit it in
src/tools somewhere.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Windows crash / abort handling
@ 2022-01-29 21:02 Andres Freund <andres@anarazel.de>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 12+ messages in thread
From: Andres Freund @ 2022-01-29 21:02 UTC (permalink / raw)
To: Craig Ringer <craig.ringer@enterprisedb.com>; +Cc: pgsql-hackers; Andrew Dunstan <andrew@dunslane.net>; Tom Lane <tgl@sss.pgh.pa.us>
Hi,
On 2022-01-09 16:57:04 -0800, Andres Freund wrote:
> I've attached a patch implementing these changes.
Unless somebody is planning to look at this soon, I'm planning to push it to
master. It's too annoying to have these hangs and not see backtraces.
We're going to have to do this in all binaries at some point, but that's
a considerably larger patch...
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Windows crash / abort handling
@ 2022-02-01 22:24 Thomas Munro <thomas.munro@gmail.com>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 12+ messages in thread
From: Thomas Munro @ 2022-02-01 22:24 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Craig Ringer <craig.ringer@enterprisedb.com>; pgsql-hackers; Andrew Dunstan <andrew@dunslane.net>; Tom Lane <tgl@sss.pgh.pa.us>
On Sun, Jan 30, 2022 at 10:02 AM Andres Freund <andres@anarazel.de> wrote:
> On 2022-01-09 16:57:04 -0800, Andres Freund wrote:
> > I've attached a patch implementing these changes.
>
> Unless somebody is planning to look at this soon, I'm planning to push it to
> master. It's too annoying to have these hangs and not see backtraces.
+1, I don't know enough about Windows development to have an opinion
on the approach but we've got to try *something*, these hangs are
terrible.
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Windows crash / abort handling
@ 2022-02-03 02:38 Andres Freund <andres@anarazel.de>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 12+ messages in thread
From: Andres Freund @ 2022-02-03 02:38 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Craig Ringer <craig.ringer@enterprisedb.com>; pgsql-hackers; Andrew Dunstan <andrew@dunslane.net>; Tom Lane <tgl@sss.pgh.pa.us>
Hi,
On 2022-02-02 11:24:19 +1300, Thomas Munro wrote:
> On Sun, Jan 30, 2022 at 10:02 AM Andres Freund <andres@anarazel.de> wrote:
> > On 2022-01-09 16:57:04 -0800, Andres Freund wrote:
> > > I've attached a patch implementing these changes.
> >
> > Unless somebody is planning to look at this soon, I'm planning to push it to
> > master. It's too annoying to have these hangs and not see backtraces.
>
> +1, I don't know enough about Windows development to have an opinion
> on the approach but we've got to try *something*, these hangs are
> terrible.
I've pushed the patch this thread is about now. Lets see what the buildfarm
says. I only could one windows version. Separately I've also pushed a patch
to run the windows tests under a timeout. I hope in combination these patches
address the hangs.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Windows crash / abort handling
@ 2022-02-04 03:21 David Rowley <dgrowleyml@gmail.com>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 12+ messages in thread
From: David Rowley @ 2022-02-04 03:21 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Thomas Munro <thomas.munro@gmail.com>; Craig Ringer <craig.ringer@enterprisedb.com>; pgsql-hackers; Andrew Dunstan <andrew@dunslane.net>; Tom Lane <tgl@sss.pgh.pa.us>
On Thu, 3 Feb 2022 at 15:38, Andres Freund <andres@anarazel.de> wrote:
> I've pushed the patch this thread is about now. Lets see what the buildfarm
> says. I only could one windows version. Separately I've also pushed a patch
> to run the windows tests under a timeout. I hope in combination these patches
> address the hangs.
I tried this out today on a windows machine I have here. I added some
code to trigger an Assert failure and the options of attaching a
debugger work well. Tested both from running the regression tests and
running a query manually with psql.
Tested on Windows 8.1 with VS2017.
Thanks for working on this.
David
^ permalink raw reply [nested|flat] 12+ messages in thread
end of thread, other threads:[~2022-02-04 03:21 UTC | newest]
Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-10-05 19:30 Windows crash / abort handling Andres Freund <andres@anarazel.de>
2021-10-06 06:11 ` Craig Ringer <craig.ringer@enterprisedb.com>
2022-01-10 00:57 ` Andres Freund <andres@anarazel.de>
2022-01-29 21:02 ` Andres Freund <andres@anarazel.de>
2022-02-01 22:24 ` Thomas Munro <thomas.munro@gmail.com>
2022-02-03 02:38 ` Andres Freund <andres@anarazel.de>
2022-02-04 03:21 ` David Rowley <dgrowleyml@gmail.com>
2022-01-10 15:57 ` Andrew Dunstan <andrew@dunslane.net>
2022-01-11 07:51 ` Andres Freund <andres@anarazel.de>
2022-01-11 17:01 ` Andrew Dunstan <andrew@dunslane.net>
2022-01-11 21:13 ` Andres Freund <andres@anarazel.de>
2022-01-11 21:35 ` Andrew Dunstan <andrew@dunslane.net>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox