Snapshot leak warning with lo_export in subtransaction

Поиск
Список
Период
Сортировка
От Heikki Linnakangas
Тема Snapshot leak warning with lo_export in subtransaction
Дата
Msg-id 32bf767a-2d65-71c4-f170-122f416bab7e@iki.fi
обсуждение исходный текст
Ответы Re: Snapshot leak warning with lo_export in subtransaction  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
Список pgsql-bugs
Hi,

Andrew B reported this warning off-list:

postgres=# SELECT lo_create(41174);
  lo_create
-----------
      41174
(1 row)

postgres=# DO $$
BEGIN
   PERFORM lo_export(41174, '/invalid/path');
EXCEPTION
   WHEN others THEN RAISE NOTICE 'error: %', sqlerrm;
END;
$$;
NOTICE:  error: could not create server file "/invalid/path": No such 
file or directory
WARNING:  Snapshot reference leak: Snapshot 0x5634afd61cb8 still referenced
DO

The code in be_lo_export does this:

> 
>     CreateFSContext();
> 
>     /*
>      * open the inversion object (no need to test for failure)
>      */
>     lobj = inv_open(lobjId, INV_READ, fscxt);
> 
>     /*
>      * open the file to be written to
>      *
>     ...

And inv_open does this:

>     /* OK to create a descriptor */
>     retval = (LargeObjectDesc *) MemoryContextAlloc(mcxt,
>                                                     sizeof(LargeObjectDesc));
>     retval->id = lobjId;
>     retval->subid = GetCurrentSubTransactionId();
>     retval->offset = 0;
>     retval->flags = descflags;
> 
>     /*
>      * We must register the snapshot in TopTransaction's resowner, because it
>      * must stay alive until the LO is closed rather than until the current
>      * portal shuts down.  Do this last to avoid uselessly leaking the
>      * snapshot if an error is thrown above.
>      */
>     if (snapshot)
>         snapshot = RegisterSnapshotOnOwner(snapshot,
>                                            TopTransactionResourceOwner);
>     retval->snapshot = snapshot;

So this is pretty clear-cut: if opening the file fails, the snapshot 
reference is leaked in TopTransactionResourceOwner. Similarly, the 
LargeObjectDesc is leaked in 'fscxt', which is a subcontext of 
TopMemoryContext, but that doesn't generate a warning.

I propose the attached patch to fix that. With the patch, we use 
CurrentMemoryContext and no resource owner for transient 
LargeObjectDescs that are opened and closed in the same function call.

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.

- Heikki

Вложения

В списке pgsql-bugs по дате отправления:

Предыдущее
От: Noah Misch
Дата:
Сообщение: Re: CREATE INDEX CONCURRENTLY does not index prepared xact's data
Следующее
От: Alvaro Herrera
Дата:
Сообщение: Re: Snapshot leak warning with lo_export in subtransaction