Re: pg11+: pg_ls_*dir LIMIT 1: temporary files .. not closed at end-of-transaction

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, David Steele <david(at)pgmasters(dot)net>, "Bossart, Nathan" <bossartn(at)amazon(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Subject: Re: pg11+: pg_ls_*dir LIMIT 1: temporary files .. not closed at end-of-transaction
Date: 2020-03-08 19:40:09
Message-ID: 5679.1583696409@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Justin Pryzby <pryzby(at)telsasoft(dot)com> writes:
> On Sun, Mar 08, 2020 at 02:37:49PM -0400, Tom Lane wrote:
>> I guess we ought to change that function to use returns-a-tuplestore
>> protocol instead of thinking it can hold a directory open across calls.
>> It's not hard to think of use-cases where the existing behavior would
>> cause issues worse than a nanny-ish WARNING, especially on platforms
>> with tight "ulimit -n" limits.

> Thanks for the analysis.

> Do you mean it should enumerate all files during the initial SRF call, or use
> something other than the SRF_* macros ?

It has to enumerate all the files during the first call. I suppose it
could do that and then still hand back the results one-at-a-time, but
there seems little point compared to filling a tuplestore immediately.
So probably the SRF_ macros are useless here.

Another possible solution is to register an exprstate-shutdown hook to
ensure the resource is cleaned up, but I'm not very happy with that
because it does nothing to prevent the hazard of overrunning the
available resources if you have several of these active at once.

I've just finished scanning the source code and concluding that all
of these functions are similarly broken:

pg_ls_dir
pg_ls_dir_files
pg_tablespace_databases
pg_logdir_ls_internal
pg_timezone_names
pgrowlocks

The first five risk leaking an open directory, the last risks leaking
an active tablescan and open relation.

I don't see anything in the documentation (either funcapi.h or
xfunc.sgml) warning that the function might not get run to completion,
either ...

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2020-03-08 20:30:44 Re: pg11+: pg_ls_*dir LIMIT 1: temporary files .. not closed at end-of-transaction
Previous Message Dean Rasheed 2020-03-08 19:17:10 Re: Additional improvements to extended statistics