Index: src/backend/storage/file/fd.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/storage/file/fd.c,v retrieving revision 1.150 diff -c -r1.150 fd.c *** src/backend/storage/file/fd.c 5 Aug 2009 18:01:54 -0000 1.150 --- src/backend/storage/file/fd.c 2 Dec 2009 21:42:01 -0000 *************** *** 55,60 **** --- 55,61 ---- #include "storage/fd.h" #include "storage/ipc.h" #include "utils/guc.h" + #include "utils/resowner.h" /* *************** *** 122,140 **** /* these are the assigned bits in fdstate below: */ #define FD_TEMPORARY (1 << 0) /* T = delete when closed */ - #define FD_XACT_TEMPORARY (1 << 1) /* T = delete at eoXact */ - - /* - * Flag to tell whether it's worth scanning VfdCache looking for temp files to - * close - */ - static bool have_xact_temporary_files = false; 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 */ File nextFree; /* link to next free VFD, if in freelist */ File lruMoreRecently; /* doubly linked recency-of-use list */ File lruLessRecently; --- 123,134 ---- /* these are the assigned bits in fdstate below: */ #define FD_TEMPORARY (1 << 0) /* T = delete when closed */ typedef struct vfd { int fd; /* current FD, or VFD_CLOSED if none */ unsigned short fdstate; /* bitflags for VFD's state */ ! 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,870 **** --- 859,865 ---- vfdP->fileMode = fileMode; vfdP->seekPos = 0; vfdP->fdstate = 0x0; + vfdP->resowner = NULL; return file; } *************** *** 876,883 **** * 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. --- 871,878 ---- * 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 associate the file with CurrentResourceOwner. ! * 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. *************** *** 918,931 **** /* Mark it for deletion at close */ VfdCache[file].fdstate |= FD_TEMPORARY; ! /* Mark it for deletion at EOXact */ if (!interXact) { ! VfdCache[file].fdstate |= FD_XACT_TEMPORARY; ! VfdCache[file].create_subid = GetCurrentSubTransactionId(); ! ! /* ensure cleanup happens at eoxact */ ! have_xact_temporary_files = true; } return file; --- 913,924 ---- /* Mark it for deletion at close */ VfdCache[file].fdstate |= FD_TEMPORARY; ! /* Associate it with the current resource owner */ if (!interXact) { ! ResourceOwnerEnlargeFiles(CurrentResourceOwner); ! ResourceOwnerRememberFile(CurrentResourceOwner, file); ! VfdCache[file].resowner = CurrentResourceOwner; } return file; *************** *** 1051,1056 **** --- 1044,1052 ---- elog(LOG, "could not unlink file \"%s\": %m", vfdP->fileName); } + if (vfdP->resowner) + ResourceOwnerForgetFile(vfdP->resowner, file); + /* * Return the Vfd slot to the free list */ *************** *** 1695,1718 **** { 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) --- 1691,1696 ---- *************** *** 1732,1746 **** * AtEOXact_Files * * 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. */ void AtEOXact_Files(void) { ! CleanupTempFiles(false); tempTableSpaces = NULL; numTempTableSpaces = -1; } --- 1710,1725 ---- * AtEOXact_Files * * This routine is called during transaction commit or abort (it doesn't ! * particularly care which). All "allocated" stdio files are closed. * We also forget any transaction-local temp tablespace list. */ void AtEOXact_Files(void) { ! /* Clean up "allocated" stdio files and dirs. */ ! while (numAllocatedDescs > 0) ! FreeDesc(&allocatedDescs[0]); ! tempTableSpaces = NULL; numTempTableSpaces = -1; } *************** *** 1749,1802 **** * AtProcExit_Files * * on_proc_exit hook to clean up temp files during backend shutdown. ! * Here, we want to clean up *all* temp files including interXact ones. */ static void AtProcExit_Files(int code, Datum arg) { ! CleanupTempFiles(true); ! } ! ! /* ! * Close temporary files and delete their underlying files. ! * ! * isProcExit: if true, this is being called as the backend process is ! * exiting. If that's the case, we should remove all temporary files; if ! * that's not the case, we are being called for transaction commit/abort ! * and should only remove transaction-local temp files. In either case, ! * also clean up "allocated" stdio files and dirs. ! */ ! static void ! CleanupTempFiles(bool isProcExit) ! { ! Index i; /* ! * Careful here: at proc_exit we need extra cleanup, not just ! * xact_temporary files. */ ! if (isProcExit || 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_TEMPORARY) && VfdCache[i].fileName != NULL) ! { ! /* ! * 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. ! */ ! if (isProcExit || (fdstate & FD_XACT_TEMPORARY)) ! FileClose(i); ! } ! } ! have_xact_temporary_files = false; } while (numAllocatedDescs > 0) FreeDesc(&allocatedDescs[0]); } --- 1728,1755 ---- * AtProcExit_Files * * on_proc_exit hook to clean up temp files during backend shutdown. ! * Here, we want to clean up *all* temp files and "allocated" stdio files ! * and dirs. */ static void AtProcExit_Files(int code, Datum arg) { ! int i; /* ! * Close all still-open temporary file VFD, which also causes the ! * underlying files to be deleted. */ ! Assert(FileIsNotOpen(0)); /* Make sure ring not corrupted */ ! for (i = 1; i < SizeVfdCache; i++) { ! unsigned short fdstate = VfdCache[i].fdstate; ! if ((fdstate & FD_TEMPORARY) && VfdCache[i].fileName != NULL) ! FileClose(i); } + /* Clean up "allocated" stdio files and dirs. */ while (numAllocatedDescs > 0) FreeDesc(&allocatedDescs[0]); } Index: src/backend/utils/resowner/resowner.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/utils/resowner/resowner.c,v retrieving revision 1.32 diff -c -r1.32 resowner.c *** src/backend/utils/resowner/resowner.c 11 Jun 2009 14:49:06 -0000 1.32 --- src/backend/utils/resowner/resowner.c 2 Dec 2009 21:42:01 -0000 *************** *** 72,77 **** --- 72,82 ---- 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,110 **** --- 110,116 ---- static void PrintPlanCacheLeakWarning(CachedPlan *plan); static void PrintTupleDescLeakWarning(TupleDesc tupdesc); static void PrintSnapshotLeakWarning(Snapshot snapshot); + static void PrintFileLeakWarning(File file); /***************************************************************************** *************** *** 316,321 **** --- 322,335 ---- 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,352 **** --- 361,367 ---- 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,382 **** --- 392,399 ---- pfree(owner->tupdescs); if (owner->snapshots) pfree(owner->snapshots); + if (owner->files) + pfree(owner->files); pfree(owner); } *************** *** 1035,1037 **** --- 1052,1138 ---- "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); + } Index: src/include/utils/resowner.h =================================================================== RCS file: /cvsroot/pgsql/src/include/utils/resowner.h,v retrieving revision 1.17 diff -c -r1.17 resowner.h *** src/include/utils/resowner.h 1 Jan 2009 17:24:02 -0000 1.17 --- src/include/utils/resowner.h 2 Dec 2009 21:42:01 -0000 *************** *** 20,25 **** --- 20,26 ---- #define RESOWNER_H #include "storage/buf.h" + #include "storage/fd.h" #include "utils/catcache.h" #include "utils/plancache.h" #include "utils/snapshot.h" *************** *** 129,132 **** --- 130,140 ---- 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 */