Hi,
The attached testcase demonstrates that it currently is possible that
buffile.c segments get created belonging to the wrong resource owner
leading to WARNINGs ala "temporary file leak: File %d still referenced",
ERRORs like "write failed", asserts and segfaults.
The problem is that while BufFileCreateTemp() callers like tuplestore
take care to use proper resource owners for it, they don't during
BufFileWrite()->BufFileDumpBuffer()->extendBufFile(). The last in that
chain creates a new tempfile which then gets owned by
CurrentResourceOwner. Which, like in the testcase, might a
subtransaction's one.
While not particularly nice, given the API, it seems best for buffile.c
to remember the resource owner used for the original segment and
temporarily set that during the extension.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/storage/file/buffile.c b/src/backend/storage/file/buffile.c
index ac8cd1d..feb7011 100644
--- a/src/backend/storage/file/buffile.c
+++ b/src/backend/storage/file/buffile.c
@@ -38,6 +38,7 @@
#include "storage/fd.h"
#include "storage/buffile.h"
#include "storage/buf_internals.h"
+#include "utils/resowner.h"
/*
* We break BufFiles into gigabyte-sized segments, regardless of RELSEG_SIZE.
@@ -67,6 +68,7 @@ struct BufFile
bool isTemp; /* can only add files if this is TRUE */
bool isInterXact; /* keep open over transactions? */
bool dirty; /* does buffer need to be written? */
+ ResourceOwner resowner; /* Resowner to use when extending */
/*
* "current pos" is position of start of buffer within the logical file.
@@ -118,11 +120,21 @@ static void
extendBufFile(BufFile *file)
{
File pfile;
+ ResourceOwner saved = CurrentResourceOwner;
+
+ /*
+ * Temporarily set resource owner to the one used for
+ * BufFileCreateTemp so the new segment has the same lifetime as
+ * the first.
+ */
+ CurrentResourceOwner = file->resowner;
Assert(file->isTemp);
pfile = OpenTemporaryFile(file->isInterXact);
Assert(pfile >= 0);
+ CurrentResourceOwner = saved;
+
file->files = (File *) repalloc(file->files,
(file->numFiles + 1) * sizeof(File));
file->offsets = (off_t *) repalloc(file->offsets,
@@ -155,7 +167,7 @@ BufFileCreateTemp(bool interXact)
file = makeBufFile(pfile);
file->isTemp = true;
file->isInterXact = interXact;
-
+ file->resowner = CurrentResourceOwner;
return file;
}
SET work_mem = '100MB';
DROP TABLE IF EXISTS data CASCADE;
CREATE TABLE data(data text);
CREATE FUNCTION leak_file() RETURNS SETOF data LANGUAGE plpgsql AS $$
DECLARE
i int;
BEGIN
FOR i IN 1..3 LOOP
DECLARE
r record;
BEGIN
-- make sure we need to extend a tempfile past one segment in a
subtransaction
FOR i IN 1..300000 LOOP
r := ('('||repeat('quite a bit of stupid text',
100)||')')::data;
RETURN NEXT r;
END LOOP;
RAISE EXCEPTION 'frakkedifrak';
EXCEPTION WHEN others THEN
RAISE NOTICE 'got exception %', sqlerrm;
END;
END LOOP;
END
$$
COPY (SELECT * FROM leak_file()) TO '/dev/null';
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
