Re: Memory leak with CALL to Procedure with COMMIT. - Mailing list pgsql-hackers

From Peter Eisentraut
Subject Re: Memory leak with CALL to Procedure with COMMIT.
Date
Msg-id 2daf4ac0-3376-673c-3858-2fc191a010b9@2ndquadrant.com
Whole thread Raw
In response to Re: Memory leak with CALL to Procedure with COMMIT.  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
Responses Re: Memory leak with CALL to Procedure with COMMIT.
Re: Memory leak with CALL to Procedure with COMMIT.
List pgsql-hackers
On 14/08/2018 15:35, Peter Eisentraut wrote:
> The commit that started this is
> 
> commit 59a85323d9d5927a852939fa93f09d142c72c91a
> Author: Peter Eisentraut <peter_e@gmx.net>
> Date:   Mon Jul 9 13:58:08 2018
> 
>     Add UtilityReturnsTuples() support for CALL
> 
>     This ensures that prepared statements for CALL can return tuples.
> 
> The change whether a statement returns tuples or not causes some
> different paths being taking deep in the portal code that set snapshot
> in different ways.  I haven't fully understood them yet.

The problem arises with the combination of CALL with output parameters
and doing a COMMIT inside the procedure.

When a CALL has output parameters, the portal uses the strategy
PORTAL_UTIL_SELECT instead of PORTAL_MULTI_QUERY.  Using
PORTAL_UTIL_SELECT causes the portal's snapshot to be registered with
the current resource owner (portal->holdSnapshot).  I'm not sure why
this is done for one kind of portal strategy but not the other.

Then, when the COMMIT inside the procedure is run, the current resource
owner is released and it complains that a snapshot is still registered
there.

There are, technically, three ways to fix this: silence the warning,
unregister the snapshot explicitly, or don't register the snapshot to
begin with.

Silencing the warning, other than by just deleting it, might require
passing in some more context information into ResourceOwnerRelease().
(The existing isTopLevel isn't quite the right thing.)

Unregistering the snapshot explicitly looks tricky because in
SPI_commit() or thereabouts we have no context information about which
snapshot might have been registered where.

Not registering the snapshot to begin with seems dubious, but see my
question above about why this is not done in the case of PORTAL_MULTI_QUERY.

Thoughts?

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: A slightly misleading comment in GetNewObjectId()
Next
From: Tom Lane
Date:
Subject: Re: C99 compliance for src/port/snprintf.c