public inbox for pgsql-bugs@postgresql.org
help / color / mirror / Atom feedFrom: Alexander Lakhin <exclusion@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18374: Printing memory contexts on OOM condition might lead to segmentation fault
Date: Mon, 4 Mar 2024 18:00:00 +0300
Message-ID: <37001bcc-7341-526c-a94d-26169f6f7282@gmail.com> (raw)
In-Reply-To: <3399097.1709501969@sss.pgh.pa.us>
References: <18374-ebb8113ce4d02f0d@postgresql.org>
<3120721.1709395887@sss.pgh.pa.us>
<b1a1eaf3-d5b7-da52-6bb7-c5b3fbe47f3e@gmail.com>
<3140126.1709405398@sss.pgh.pa.us>
<3148162.1709409519@sss.pgh.pa.us>
<3399097.1709501969@sss.pgh.pa.us>
Hello Tom,
04.03.2024 00:39, Tom Lane wrote:
> I wrote:
>> I find this in [1]:
>>
>> The C language stack growth does an implicit mremap. If you want absolute
>> guarantees and run close to the edge you MUST mmap your stack for the
>> largest size you think you will need. For typical stack usage this does
>> not matter much but it's a corner case if you really really care
>>
>> Seems like we need to do some more work at startup to enforce that
>> we have the amount of stack we think we do, if we're on Linux.
> After thinking about that some more, I'm really quite unenthused about
> trying to remap the stack for ourselves. It'd be both platform- and
> architecture-dependent, and I'm afraid it'd introduce as many failure
> modes as it removes. (Notably, I'm not sure we could guarantee
> there's a guard page below the stack.) Since we've not seen reports
> of this failure from the wild, I doubt it's worth the trouble.
I have perhaps a naive idea, but it apparently eliminates the segmentation
fault for the given test case. (Please look at a quick draft attached.)
Though maybe the issue can really wait for complaints from outside or for
a simpler/cheaper solution, integrated with the existing (or future)
stack-overflow protection.
> I do think it's probably worth reducing MemoryContextDelete's stack
> usage to O(1), just to ensure we can't get into stack trouble during
> transaction abort. That's not hard at all, as attached.
Yeah, Heikki proposed something similar as part of
0003-Avoid-recursion-in-MemoryContext-functions.patch there:
https://www.postgresql.org/message-id/6b48c746-9704-46dc-b9be-01fe4137c824%40iki.fi
> I tried to make MemoryContextResetChildren work similarly, but that
> doesn't work because if we're not removing child contexts then we
> need extra state to tell which ones we've done already. For the
> same reason my idea for bounding the stack space needed by
> MemoryContextStats doesn't seem to work. We could possibly make it
> work if we were willing to add a temporary-use pointer field to all
> MemoryContext headers, but I'm unconvinced that'd be a good tradeoff.
Best regards,
Alexander
Attachments:
[text/x-patch] acquire-memory-for-stack.patch (656B, 2-acquire-memory-for-stack.patch)
download | inline diff:
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 59ab812d2e..bb9d309f4e 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3464,6 +3464,14 @@ ProcessInterrupts(void)
HandleParallelApplyMessages();
}
+void prepare_stack(int n)
+{
+ char buffer[1024 * 1024];
+ memset(buffer, 0, sizeof(buffer));
+ if (n > 0)
+ prepare_stack(n - 1);
+}
+
/*
* set_stack_base: set up reference point for stack depth checking
*
@@ -3490,6 +3498,7 @@ set_stack_base(void)
stack_base_ptr = &stack_base;
#endif
+ prepare_stack((max_stack_depth_bytes + STACK_DEPTH_SLOP) / (1024 * 1024));
return old;
}
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: exclusion@gmail.com, tgl@sss.pgh.pa.us, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18374: Printing memory contexts on OOM condition might lead to segmentation fault
In-Reply-To: <37001bcc-7341-526c-a94d-26169f6f7282@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox