diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index e4ebb93fc92..3e3932b861e 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.121.2.3 2006/01/17 23:52:50 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/storage/file/fd.c,v 1.121.2.4 2009/12/03 11:04:02 heikki Exp $ * * NOTES: * @@ -50,6 +50,7 @@ #include "access/xact.h" #include "storage/fd.h" #include "storage/ipc.h" +#include "utils/resowner.h" /* @@ -123,7 +124,7 @@ typedef struct vfd { signed short 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; @@ -839,6 +840,7 @@ PathNameOpenFile(FileName fileName, int fileFlags, int fileMode) vfdP->fileMode = fileMode; vfdP->seekPos = 0; vfdP->fdstate = 0x0; + vfdP->resowner = NULL; return file; } @@ -868,11 +870,12 @@ FileNameOpenFile(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) @@ -922,11 +925,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; } return file; @@ -973,6 +979,10 @@ FileClose(File file) vfdP->fileName); } + /* Unregister it from the resource owner */ + if (vfdP->resowner) + ResourceOwnerForgetFile(vfdP->resowner, file); + /* * Return the Vfd slot to the free list */ @@ -1515,24 +1525,6 @@ AtEOSubXact_Files(bool isCommit, SubTransactionId mySubid, { Index i; - if (SizeVfdCache > 0) - { - 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) @@ -1553,8 +1545,9 @@ 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. + * 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. */ void AtEOXact_Files(void) @@ -1600,14 +1593,24 @@ 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); + } } } } + /* 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 fb01b1a5fba..573d1d4b459 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.14.2.2 2005/12/08 19:19:31 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/resowner/resowner.c,v 1.14.2.3 2009/12/03 11:04:03 heikki Exp $ * *------------------------------------------------------------------------- */ @@ -58,6 +58,11 @@ typedef struct ResourceOwnerData int nrelrefs; /* number of owned relcache pins */ Relation *relrefs; /* dynamically allocated array */ int maxrelrefs; /* 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; @@ -88,6 +93,7 @@ static void ResourceOwnerReleaseInternal(ResourceOwner owner, bool isCommit, bool isTopLevel); static void PrintRelCacheLeakWarning(Relation rel); +static void PrintFileLeakWarning(File file); /***************************************************************************** @@ -277,6 +283,14 @@ ResourceOwnerReleaseInternal(ResourceOwner owner, ReleaseCatCacheList(owner->catlistrefs[owner->ncatlistrefs - 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_gist(); ReleaseResources_hash(); @@ -307,6 +321,7 @@ ResourceOwnerDelete(ResourceOwner owner) Assert(owner->ncatrefs == 0); Assert(owner->ncatlistrefs == 0); Assert(owner->nrelrefs == 0); + Assert(owner->nfiles == 0); /* * Delete children. The recursive call will delink the child from me, so @@ -331,6 +346,8 @@ ResourceOwnerDelete(ResourceOwner owner) pfree(owner->catlistrefs); if (owner->relrefs) pfree(owner->relrefs); + if (owner->files) + pfree(owner->files); pfree(owner); } @@ -745,3 +762,87 @@ PrintRelCacheLeakWarning(Relation rel) elog(WARNING, "relcache reference leak: relation \"%s\" not closed", RelationGetRelationName(rel)); } + + +/* + * 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 11e6f4eee2d..28d1ca51627 100644 --- a/src/include/utils/resowner.h +++ b/src/include/utils/resowner.h @@ -12,7 +12,7 @@ * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/utils/resowner.h,v 1.5 2004/12/31 22:03:46 pgsql Exp $ + * $PostgreSQL: pgsql/src/include/utils/resowner.h,v 1.5.6.1 2009/12/03 11:04:03 heikki Exp $ * *------------------------------------------------------------------------- */ @@ -20,6 +20,7 @@ #define RESOWNER_H #include "storage/buf.h" +#include "storage/fd.h" #include "utils/catcache.h" #include "utils/rel.h" @@ -107,4 +108,11 @@ extern void ResourceOwnerRememberRelationRef(ResourceOwner owner, extern void ResourceOwnerForgetRelationRef(ResourceOwner owner, Relation rel); +/* 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 */