diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index b608b2c311b2beccaaafcb9135680780b2da22dc..32979229d8fbc9b35afb065a249b6f22dc7210a4 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/storage/file/fd.c,v 1.150 2009/08/05 18:01:54 heikki Exp $ + * $PostgreSQL: pgsql/src/backend/storage/file/fd.c,v 1.151 2009/12/03 11:03:28 heikki Exp $ * * NOTES: * @@ -55,6 +55,7 @@ #include "storage/fd.h" #include "storage/ipc.h" #include "utils/guc.h" +#include "utils/resowner.h" /* @@ -134,7 +135,7 @@ typedef struct vfd { int fd; /* current FD, or VFD_CLOSED if none */ unsigned short fdstate; /* bitflags for VFD's state */ - SubTransactionId create_subid; /* for TEMPORARY fds, creating subxact */ + ResourceOwner resowner; /* owner, for automatic cleanup */ File nextFree; /* link to next free VFD, if in freelist */ File lruMoreRecently; /* doubly linked recency-of-use list */ File lruLessRecently; @@ -865,6 +866,7 @@ PathNameOpenFile(FileName fileName, int fileFlags, int fileMode) vfdP->fileMode = fileMode; vfdP->seekPos = 0; vfdP->fdstate = 0x0; + vfdP->resowner = NULL; return file; } @@ -876,11 +878,12 @@ PathNameOpenFile(FileName fileName, int fileFlags, int fileMode) * There's no need to pass in fileFlags or fileMode either, since only * one setting makes any sense for a temp file. * - * interXact: if true, don't close the file at end-of-transaction. In - * most cases, you don't want temporary files to outlive the transaction - * that created them, so this should be false -- but if you need - * "somewhat" temporary storage, this might be useful. In either case, - * the file is removed when the File is explicitly closed. + * Unless interXact is true, the file is remembered by CurrentResourceOwner + * to ensure it's closed and deleted when it's no longer needed, typically at + * the end-of-transaction. In most cases, you don't want temporary files to + * outlive the transaction that created them, so this should be false -- but + * if you need "somewhat" temporary storage, this might be useful. In either + * case, the file is removed when the File is explicitly closed. */ File OpenTemporaryFile(bool interXact) @@ -918,11 +921,14 @@ OpenTemporaryFile(bool interXact) /* Mark it for deletion at close */ VfdCache[file].fdstate |= FD_TEMPORARY; - /* Mark it for deletion at EOXact */ + /* Register it with the current resource owner */ if (!interXact) { VfdCache[file].fdstate |= FD_XACT_TEMPORARY; - VfdCache[file].create_subid = GetCurrentSubTransactionId(); + + ResourceOwnerEnlargeFiles(CurrentResourceOwner); + ResourceOwnerRememberFile(CurrentResourceOwner, file); + VfdCache[file].resowner = CurrentResourceOwner; /* ensure cleanup happens at eoxact */ have_xact_temporary_files = true; @@ -1051,6 +1057,10 @@ FileClose(File file) elog(LOG, "could not unlink file \"%s\": %m", vfdP->fileName); } + /* Unregister it from the resource owner */ + if (vfdP->resowner) + ResourceOwnerForgetFile(vfdP->resowner, file); + /* * Return the Vfd slot to the free list */ @@ -1695,24 +1705,6 @@ AtEOSubXact_Files(bool isCommit, SubTransactionId mySubid, { Index i; - if (have_xact_temporary_files) - { - Assert(FileIsNotOpen(0)); /* Make sure ring not corrupted */ - for (i = 1; i < SizeVfdCache; i++) - { - unsigned short fdstate = VfdCache[i].fdstate; - - if ((fdstate & FD_XACT_TEMPORARY) && - VfdCache[i].create_subid == mySubid) - { - if (isCommit) - VfdCache[i].create_subid = parentSubid; - else if (VfdCache[i].fileName != NULL) - FileClose(i); - } - } - } - for (i = 0; i < numAllocatedDescs; i++) { if (allocatedDescs[i].create_subid == mySubid) @@ -1733,9 +1725,10 @@ AtEOSubXact_Files(bool isCommit, SubTransactionId mySubid, * * This routine is called during transaction commit or abort (it doesn't * particularly care which). All still-open per-transaction temporary file - * VFDs are closed, which also causes the underlying files to be - * deleted. Furthermore, all "allocated" stdio files are closed. - * We also forget any transaction-local temp tablespace list. + * VFDs are closed, which also causes the underlying files to be deleted + * (although they should've been closed already by the ResourceOwner + * cleanup). Furthermore, all "allocated" stdio files are closed. We also + * forget any transaction-local temp tablespace list. */ void AtEOXact_Files(void) @@ -1787,16 +1780,26 @@ CleanupTempFiles(bool isProcExit) /* * If we're in the process of exiting a backend process, close * all temporary files. Otherwise, only close temporary files - * local to the current transaction. + * local to the current transaction. They should be closed + * by the ResourceOwner mechanism already, so this is just + * a debugging cross-check. */ - if (isProcExit || (fdstate & FD_XACT_TEMPORARY)) + if (isProcExit) + FileClose(i); + else if (fdstate & FD_XACT_TEMPORARY) + { + elog(WARNING, + "temporary file %s not closed at end-of-transaction", + VfdCache[i].fileName); FileClose(i); + } } } have_xact_temporary_files = false; } + /* Clean up "allocated" stdio files and dirs. */ while (numAllocatedDescs > 0) FreeDesc(&allocatedDescs[0]); } diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c index 4ef47ca5601b33c686ecc0a4b36b58304ceaf2bb..1f8658d010403ed6816e102ba72dedb509deabb4 100644 --- a/src/backend/utils/resowner/resowner.c +++ b/src/backend/utils/resowner/resowner.c @@ -14,7 +14,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/resowner/resowner.c,v 1.32 2009/06/11 14:49:06 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/utils/resowner/resowner.c,v 1.33 2009/12/03 11:03:29 heikki Exp $ * *------------------------------------------------------------------------- */ @@ -72,6 +72,11 @@ typedef struct ResourceOwnerData int nsnapshots; /* number of owned snapshot references */ Snapshot *snapshots; /* dynamically allocated array */ int maxsnapshots; /* currently allocated array size */ + + /* We have built-in support for remembering open temporary files */ + int nfiles; /* number of owned temporary files */ + File *files; /* dynamically allocated array */ + int maxfiles; /* currently allocated array size */ } ResourceOwnerData; @@ -105,6 +110,7 @@ static void PrintRelCacheLeakWarning(Relation rel); static void PrintPlanCacheLeakWarning(CachedPlan *plan); static void PrintTupleDescLeakWarning(TupleDesc tupdesc); static void PrintSnapshotLeakWarning(Snapshot snapshot); +static void PrintFileLeakWarning(File file); /***************************************************************************** @@ -316,6 +322,14 @@ ResourceOwnerReleaseInternal(ResourceOwner owner, UnregisterSnapshot(owner->snapshots[owner->nsnapshots - 1]); } + /* Ditto for temporary files */ + while (owner->nfiles > 0) + { + if (isCommit) + PrintFileLeakWarning(owner->files[owner->nfiles - 1]); + FileClose(owner->files[owner->nfiles - 1]); + } + /* Clean up index scans too */ ReleaseResources_hash(); } @@ -347,6 +361,7 @@ ResourceOwnerDelete(ResourceOwner owner) Assert(owner->nplanrefs == 0); Assert(owner->ntupdescs == 0); Assert(owner->nsnapshots == 0); + Assert(owner->nfiles == 0); /* * Delete children. The recursive call will delink the child from me, so @@ -377,6 +392,8 @@ ResourceOwnerDelete(ResourceOwner owner) pfree(owner->tupdescs); if (owner->snapshots) pfree(owner->snapshots); + if (owner->files) + pfree(owner->files); pfree(owner); } @@ -1035,3 +1052,87 @@ PrintSnapshotLeakWarning(Snapshot snapshot) "Snapshot reference leak: Snapshot %p still referenced", snapshot); } + + +/* + * Make sure there is room for at least one more entry in a ResourceOwner's + * files reference array. + * + * This is separate from actually inserting an entry because if we run out + * of memory, it's critical to do so *before* acquiring the resource. + */ +void +ResourceOwnerEnlargeFiles(ResourceOwner owner) +{ + int newmax; + + if (owner->nfiles < owner->maxfiles) + return; /* nothing to do */ + + if (owner->files == NULL) + { + newmax = 16; + owner->files = (File *) + MemoryContextAlloc(TopMemoryContext, newmax * sizeof(File)); + owner->maxfiles = newmax; + } + else + { + newmax = owner->maxfiles * 2; + owner->files = (File *) + repalloc(owner->files, newmax * sizeof(File)); + owner->maxfiles = newmax; + } +} + +/* + * Remember that a temporary file is owned by a ResourceOwner + * + * Caller must have previously done ResourceOwnerEnlargeFiles() + */ +void +ResourceOwnerRememberFile(ResourceOwner owner, File file) +{ + Assert(owner->nfiles < owner->maxfiles); + owner->files[owner->nfiles] = file; + owner->nfiles++; +} + +/* + * Forget that a temporary file is owned by a ResourceOwner + */ +void +ResourceOwnerForgetFile(ResourceOwner owner, File file) +{ + File *files = owner->files; + int ns1 = owner->nfiles - 1; + int i; + + for (i = ns1; i >= 0; i--) + { + if (files[i] == file) + { + while (i < ns1) + { + files[i] = files[i + 1]; + i++; + } + owner->nfiles = ns1; + return; + } + } + elog(ERROR, "temporery file %d is not owned by resource owner %s", + file, owner->name); +} + + +/* + * Debugging subroutine + */ +static void +PrintFileLeakWarning(File file) +{ + elog(WARNING, + "temporary file leak: File %d still referenced", + file); +} diff --git a/src/include/utils/resowner.h b/src/include/utils/resowner.h index 4725d05687de30627c6c2a7818c981414a7c43dd..67b949cd75714cd3e052a4d98dd536ea9c37702c 100644 --- a/src/include/utils/resowner.h +++ b/src/include/utils/resowner.h @@ -12,7 +12,7 @@ * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/utils/resowner.h,v 1.17 2009/01/01 17:24:02 momjian Exp $ + * $PostgreSQL: pgsql/src/include/utils/resowner.h,v 1.18 2009/12/03 11:03:29 heikki Exp $ * *------------------------------------------------------------------------- */ @@ -20,6 +20,7 @@ #define RESOWNER_H #include "storage/buf.h" +#include "storage/fd.h" #include "utils/catcache.h" #include "utils/plancache.h" #include "utils/snapshot.h" @@ -129,4 +130,11 @@ extern void ResourceOwnerRememberSnapshot(ResourceOwner owner, extern void ResourceOwnerForgetSnapshot(ResourceOwner owner, Snapshot snapshot); +/* support for temporary file management */ +extern void ResourceOwnerEnlargeFiles(ResourceOwner owner); +extern void ResourceOwnerRememberFile(ResourceOwner owner, + File file); +extern void ResourceOwnerForgetFile(ResourceOwner owner, + File file); + #endif /* RESOWNER_H */