Server crash when describing a FETCH statement after its cursor is closed
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253940psql -h localhost -U postgresBuilt from patchset v4 (message #4), September 30, 2026 at 08:18 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253940_4 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253940_4 && git checkout t253940_4Patchset v4 (message #4) is on t253940_4
Hi,
The following extended-protocol sequence crashes a backend (SIGSEGV
in production builds, assertion failure in assert-enabled ones), and
the postmaster terminates all other sessions for crash recovery:
Query BEGIN
Query DECLARE c CURSOR FOR SELECT id FROM t ORDER BY id
Parse "" "FETCH 5 FROM c"
Bind portal "p", statement ""
Sync
Query CLOSE c
Describe portal "p"
Sync -> connection drops; server log shows
"terminated by signal 11", then
"terminating any other active
server processes"
Describing the prepared statement instead of the portal crashes the
same way. Reproduced on REL_18_STABLE (18.6) and current master,
the code is identical on all supported branches.
At Bind time, PortalStart() copies the cursor's tuple descriptor into
the portal via UtilityTupleDescriptor(), which deliberately tolerates
a missing cursor. But Describe re-resolves the target list through
FetchStatementTargetList() (tcop/pquery.c), whose FetchStmt branch does:
subportal = GetPortalByName(fstmt->portalname);
Assert(PortalIsValid(subportal));
return FetchPortalTargetList(subportal);
With the cursor closed, GetPortalByName() returns NULL and the Assert
is compiled out in production builds, so FetchPortalTargetList()
dereferences NULL->strategy. The statement variant reaches the same
branch via CachedPlanGetTargetList().
The attached patch replaces the Assert with a runtime check returning
NIL, mirroring the guard in UtilityTupleDescriptor(). A regression
test is included.
--
Cheers,
Dirkjan Bussink
Dirkjan Bussink <d.bussink@gmail.com> writes:
The following extended-protocol sequence crashes a backend (SIGSEGV
in production builds, assertion failure in assert-enabled ones), and
the postmaster terminates all other sessions for crash recovery:
Thanks for the report.
The attached patch replaces the Assert with a runtime check returning
NIL, mirroring the guard in UtilityTupleDescriptor(). A regression
test is included.
I'm not convinced by this blithe assertion "not our business to raise
error". The adjacent case for ExecuteStmt *will* raise an error if
it can't find the prepared statement; why shouldn't this case act like
that one? IOW, the sequence of client operations that you show is
invalid, and I think we ought to push back on that accordingly.
regards, tom lane
Hi,
On 25 Sep 2026, at 16:33, Tom Lane <tgl@sss.pgh.pa.us> wrote:
The attached patch replaces the Assert with a runtime check returning
NIL, mirroring the guard in UtilityTupleDescriptor(). A regression
test is included.I'm not convinced by this blithe assertion "not our business to raise
error". The adjacent case for ExecuteStmt *will* raise an error if
it can't find the prepared statement; why shouldn't this case act like
that one? IOW, the sequence of client operations that you show is
invalid, and I think we ought to push back on that accordingly.
Also happy to change this to return an error then if that is preferred?
Crashing seems very wrong, but an error seems fine?
I can change the patch then to do that.
Cheers,
Dirkjan
Hi,
On 25 Sep 2026, at 16:33, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I'm not convinced by this blithe assertion "not our business to raise
error". The adjacent case for ExecuteStmt *will* raise an error if
it can't find the prepared statement; why shouldn't this case act like
that one? IOW, the sequence of client operations that you show is
invalid, and I think we ought to push back on that accordingly.
Alright, here is a patch that now explicit returns an error when the portal
is already closed, matching what ExecuteStmt does.
--
Cheers,
Dirkjan
On Sat, Sep 26, 2026 at 09:23:42AM +0200, Dirkjan Bussink wrote:
Alright, here is a patch that now explicit returns an error when the portal
is already closed, matching what ExecuteStmt does.
Sounds acceptable to me to return an error and having a test this way.
Something that the other test subroutines of libpq_pipeline do and
that your new test subroutine does not do is the use of a fprintf() to
log the beginning of the test. That's a minor point.
libpq_pipeline is OK for the test location, we already have a few
things that don't care about pipelining like the protocol version and
its README also says that.
Another thing I was wondering is if any of the psql meta-commands
could be of some help here. However, as far as I can see, these
cannot help because psql sends more describes than what we need to
trigger the problem. Unfortunate, but again no big deal with libpq
able to wrap the job.
Tom, were you planning to double-check the contents of this thread?
--
Michael
On Thu, Oct 01, 2026 at 02:26:20PM +0900, Michael Paquier wrote:
Another thing I was wondering is if any of the psql meta-commands
could be of some help here. However, as far as I can see, these
cannot help because psql sends more describes than what we need to
trigger the problem. Unfortunate, but again no big deal with libpq
able to wrap the job.
Hearing nothing, as I had some time this morning, wrapped and done all
the way down.
--
Michael