Re: [bug fix] postgres.exe crashes with access violation on Windows while starting up

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [bug fix] postgres.exe crashes with access violation on Windows while starting up
Date: 2017-10-28 22:43:02
Message-ID: CAB7nPqTvKj_jj_zEJtRXhA2M3LLpzR29X0EU81R_EiW25zu2LQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Oct 26, 2017 at 7:10 PM, Tsunakawa, Takayuki
<tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com> wrote:
> FIX
> ==============================
>
> Add the check "CurrentMemoryContext != NULL" in write_eventlog() as in write_console().

* Also verify that we are not on our way into error recursion
trouble due
* to error messages thrown deep inside pgwin32_message_to_UTF16().
*/
if (!in_error_recursion_trouble() &&
+ CurrentMemoryContext != NULL &&
GetMessageEncoding() != GetACPEncoding())
{
So you are basically ready to lose any message that could be pushed
here if there is no memory context? That does not sound like a good
trade-off to me. A static buffer does not look like the best idea
either to not truncate message, so couldn't we envisage to just use
malloc? pgwin32_message_to_UTF16() is called in two places in elog.c,
and there is a full control on the error code paths.

> NOTE
> ==============================
>
> The reason is for not outputing the crash dump is a) the crash occurred before installing the Windows exception handler (pgwin32_install_crashdump_handler() call) and b) the effect of the following call in postmaster is inherited in the child process.
>
> /* In case of general protection fault, don't show GUI popup box */
> SetErrorMode(SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX);
>
> But I'm not sure in what order we should do pgwin32_install_crashdump_handler(), startup_hacks() and steps therein, MemoryContextInit(). I think that's another patch.

Perhaps. I don't have a final opinion on this matter.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2017-10-28 23:11:34 Re: taking stdbool.h into use
Previous Message Alexander Korotkov 2017-10-28 22:30:12 Re: CUBE seems a bit confused about ORDER BY