Re: Snapshot leak warning with lo_export in subtransaction

On 27/09/2021 15:33, Alvaro Herrera wrote:
> On 2021-Sep-27, Heikki Linnakangas wrote:
> 
>> This should be backpatched to all supported versions. This adds an argument
>> to 'inv_open' function, but I don't think there are extensions that use the
>> inv_*() functions directly. inv_api.c relies on close_lo_relation() being
>> called at the end of transaction, so I think an extension would find it hard
>> to use those functions correctly, anyway.
> 
> Hmm, but you could also use a new value in its existing 'flags' argument
> instead of changing ABI.

I tried that, but didn't like the result. It conflated the user-visible 
INV_READ/WRITE flags with the new internal-only flag.

Thinking about this some more, I came up with the attached. It moves the 
responsibility of registering the snapshot from inv_api.c to the caller. 
With that change, there's no need for a new option to inv_open(). The 
division of labor between be-fsstubs.c and inv_api.c has always been a 
bit blurry, I think that this makes it slightly more clear.

With this change, if there is an existing extension out there that calls 
inv_open(), the returned desc will be short-lived. As I said above, I 
think that's a reasonable assumption because dealing with a long-lived 
LO desc correctly would require the caller to jump through some hoops 
even before this change.

- Heikki