Ensure all files created for a single BufFile have the same resource owner.

Callers expect that they only have to set the right resource owner when
creating a BufFile, not during subsequent operations on it.  While we could
insist this be fixed at the caller level, it seems more sensible for the
BufFile to take care of it.  Without this, some temp files belonging to
a BufFile can go away too soon, eg at the end of a subtransaction,
leading to errors or crashes.

Reported and fixed by Andres Freund.  Back-patch to all active branches.
This commit is contained in:
Tom Lane 2013-11-01 16:09:48 -04:00
parent 45f64f1bbf
commit bffd1ce92c

View File

@ -23,8 +23,10 @@
* will go away automatically at transaction end. If the underlying * will go away automatically at transaction end. If the underlying
* virtual File is made with OpenTemporaryFile, then all resources for * virtual File is made with OpenTemporaryFile, then all resources for
* the file are certain to be cleaned up even if processing is aborted * the file are certain to be cleaned up even if processing is aborted
* by ereport(ERROR). To avoid confusion, the caller should take care that * by ereport(ERROR). The data structures required are made in the
* all calls for a single BufFile are made in the same palloc context. * palloc context that was current when the BufFile was created, and
* any external resources such as temp files are owned by the ResourceOwner
* that was current at that time.
* *
* BufFile also supports temporary files that exceed the OS file size limit * BufFile also supports temporary files that exceed the OS file size limit
* (by opening multiple fd.c temporary files). This is an essential feature * (by opening multiple fd.c temporary files). This is an essential feature
@ -38,6 +40,7 @@
#include "storage/fd.h" #include "storage/fd.h"
#include "storage/buffile.h" #include "storage/buffile.h"
#include "storage/buf_internals.h" #include "storage/buf_internals.h"
#include "utils/resowner.h"
/* /*
* We break BufFiles into gigabyte-sized segments, regardless of RELSEG_SIZE. * We break BufFiles into gigabyte-sized segments, regardless of RELSEG_SIZE.
@ -68,6 +71,13 @@ struct BufFile
bool isInterXact; /* keep open over transactions? */ bool isInterXact; /* keep open over transactions? */
bool dirty; /* does buffer need to be written? */ bool dirty; /* does buffer need to be written? */
/*
* resowner is the ResourceOwner to use for underlying temp files. (We
* don't need to remember the memory context we're using explicitly,
* because after creation we only repalloc our arrays larger.)
*/
ResourceOwner resowner;
/* /*
* "current pos" is position of start of buffer within the logical file. * "current pos" is position of start of buffer within the logical file.
* Position as seen by user of BufFile is (curFile, curOffset + pos). * Position as seen by user of BufFile is (curFile, curOffset + pos).
@ -103,6 +113,7 @@ makeBufFile(File firstfile)
file->isTemp = false; file->isTemp = false;
file->isInterXact = false; file->isInterXact = false;
file->dirty = false; file->dirty = false;
file->resowner = CurrentResourceOwner;
file->curFile = 0; file->curFile = 0;
file->curOffset = 0L; file->curOffset = 0L;
file->pos = 0; file->pos = 0;
@ -118,11 +129,18 @@ static void
extendBufFile(BufFile *file) extendBufFile(BufFile *file)
{ {
File pfile; File pfile;
ResourceOwner oldowner;
/* Be sure to associate the file with the BufFile's resource owner */
oldowner = CurrentResourceOwner;
CurrentResourceOwner = file->resowner;
Assert(file->isTemp); Assert(file->isTemp);
pfile = OpenTemporaryFile(file->isInterXact); pfile = OpenTemporaryFile(file->isInterXact);
Assert(pfile >= 0); Assert(pfile >= 0);
CurrentResourceOwner = oldowner;
file->files = (File *) repalloc(file->files, file->files = (File *) repalloc(file->files,
(file->numFiles + 1) * sizeof(File)); (file->numFiles + 1) * sizeof(File));
file->offsets = (off_t *) repalloc(file->offsets, file->offsets = (off_t *) repalloc(file->offsets,
@ -141,7 +159,8 @@ extendBufFile(BufFile *file)
* at end of transaction. * at end of transaction.
* *
* Note: if interXact is true, the caller had better be calling us in a * Note: if interXact is true, the caller had better be calling us in a
* memory context that will survive across transaction boundaries. * memory context, and with a resource owner, that will survive across
* transaction boundaries.
*/ */
BufFile * BufFile *
BufFileCreateTemp(bool interXact) BufFileCreateTemp(bool interXact)