From 7a57a672788ff04723544a650e33502429ad8581 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 23 Feb 2004 23:03:10 +0000 Subject: [PATCH] Replace opendir/closedir calls throughout the backend with AllocateDir and FreeDir routines modeled on the existing AllocateFile/FreeFile. Like the latter, these routines will avoid failing on EMFILE/ENFILE conditions whenever possible, and will prevent leakage of directory descriptors if an elog() occurs while one is open. Also, reduce PANIC to ERROR in MoveOfflineLogs() --- this is not critical code and there is no reason to force a DB restart on failure. All per recent trouble report from Olivier Hubaut. --- contrib/dbsize/dbsize.c | 7 +- src/backend/access/transam/slru.c | 8 +- src/backend/access/transam/xlog.c | 12 ++- src/backend/storage/file/fd.c | 126 ++++++++++++++++++++++++++---- src/include/storage/fd.h | 14 +++- src/port/copydir.c | 12 +-- 6 files changed, 138 insertions(+), 41 deletions(-) diff --git a/contrib/dbsize/dbsize.c b/contrib/dbsize/dbsize.c index 0037c14e70..7b97682237 100644 --- a/contrib/dbsize/dbsize.c +++ b/contrib/dbsize/dbsize.c @@ -1,16 +1,15 @@ #include "postgres.h" #include -#include #include #include -#include #include "access/heapam.h" #include "catalog/catalog.h" #include "catalog/namespace.h" #include "commands/dbcommands.h" #include "fmgr.h" +#include "storage/fd.h" #include "utils/builtins.h" @@ -58,7 +57,7 @@ database_size(PG_FUNCTION_ARGS) dbpath = GetDatabasePath(dbid); - dirdesc = opendir(dbpath); + dirdesc = AllocateDir(dbpath); if (!dirdesc) ereport(ERROR, (errcode_for_file_access(), @@ -93,7 +92,7 @@ database_size(PG_FUNCTION_ARGS) pfree(fullname); } - closedir(dirdesc); + FreeDir(dirdesc); PG_RETURN_INT64(totalsize); } diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c index aac93a60a3..a12fe88556 100644 --- a/src/backend/access/transam/slru.c +++ b/src/backend/access/transam/slru.c @@ -6,15 +6,13 @@ * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/backend/access/transam/slru.c,v 1.12 2004/02/17 03:45:17 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/access/transam/slru.c,v 1.13 2004/02/23 23:03:10 tgl Exp $ * *------------------------------------------------------------------------- */ #include "postgres.h" #include -#include -#include #include #include @@ -888,7 +886,7 @@ SlruScanDirectory(SlruCtl ctl, int cutoffPage, bool doDeletions) int segpage; char path[MAXPGPATH]; - cldir = opendir(ctl->Dir); + cldir = AllocateDir(ctl->Dir); if (cldir == NULL) ereport(ERROR, (errcode_for_file_access(), @@ -927,7 +925,7 @@ SlruScanDirectory(SlruCtl ctl, int cutoffPage, bool doDeletions) ereport(ERROR, (errcode_for_file_access(), errmsg("could not read directory \"%s\": %m", ctl->Dir))); - closedir(cldir); + FreeDir(cldir); return found; } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index dc6fea9b5b..821b504cbf 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/backend/access/transam/xlog.c,v 1.136 2004/02/17 03:45:17 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/access/transam/xlog.c,v 1.137 2004/02/23 23:03:10 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -17,10 +17,8 @@ #include #include #include -#include #include #include -#include #include "access/clog.h" #include "access/transam.h" @@ -1753,9 +1751,9 @@ MoveOfflineLogs(uint32 log, uint32 seg, XLogRecPtr endptr) XLByteToPrevSeg(endptr, endlogId, endlogSeg); - xldir = opendir(XLogDir); + xldir = AllocateDir(XLogDir); if (xldir == NULL) - ereport(PANIC, + ereport(ERROR, (errcode_for_file_access(), errmsg("could not open transaction log directory \"%s\": %m", XLogDir))); @@ -1812,11 +1810,11 @@ MoveOfflineLogs(uint32 log, uint32 seg, XLogRecPtr endptr) errno = 0; #endif if (errno) - ereport(PANIC, + ereport(ERROR, (errcode_for_file_access(), errmsg("could not read transaction log directory \"%s\": %m", XLogDir))); - closedir(xldir); + FreeDir(xldir); } /* diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index 2446f97b65..5ef12de949 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.107 2004/02/23 20:45:59 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/storage/file/fd.c,v 1.108 2004/02/23 23:03:10 tgl Exp $ * * NOTES: * @@ -43,8 +43,6 @@ #include #include #include -#include -#include #include #include @@ -87,11 +85,11 @@ int max_files_per_process = 1000; /* * Maximum number of file descriptors to open for either VFD entries or - * AllocateFile files. This is initialized to a conservative value, and - * remains that way indefinitely in bootstrap or standalone-backend cases. - * In normal postmaster operation, the postmaster calls set_max_safe_fds() - * late in initialization to update the value, and that value is then - * inherited by forked subprocesses. + * AllocateFile/AllocateDir operations. This is initialized to a conservative + * value, and remains that way indefinitely in bootstrap or standalone-backend + * cases. In normal postmaster operation, the postmaster calls + * set_max_safe_fds() late in initialization to update the value, and that + * value is then inherited by forked subprocesses. * * Note: the value of max_files_per_process is taken into account while * setting this variable, and so need not be tested separately. @@ -159,6 +157,17 @@ static int nfile = 0; static int numAllocatedFiles = 0; static FILE *allocatedFiles[MAX_ALLOCATED_FILES]; +/* + * List of DIRs opened with AllocateDir. + * + * Since we don't have heavy use of AllocateDir, it seems OK to put a pretty + * small maximum limit on the number of simultaneously allocated dirs. + */ +#define MAX_ALLOCATED_DIRS 10 + +static int numAllocatedDirs = 0; +static DIR *allocatedDirs[MAX_ALLOCATED_DIRS]; + /* * Number of temporary files opened during the current session; * this is used in generation of tempfile names. @@ -489,7 +498,7 @@ LruInsert(File file) if (FileIsNotOpen(file)) { - while (nfile + numAllocatedFiles >= max_safe_fds) + while (nfile + numAllocatedFiles + numAllocatedDirs >= max_safe_fds) { if (!ReleaseLruFile()) break; @@ -748,7 +757,7 @@ fileNameOpenFile(FileName fileName, file = AllocateVfd(); vfdP = &VfdCache[file]; - while (nfile + numAllocatedFiles >= max_safe_fds) + while (nfile + numAllocatedFiles + numAllocatedDirs >= max_safe_fds) { if (!ReleaseLruFile()) break; @@ -1099,8 +1108,8 @@ AllocateFile(char *name, char *mode) * looping. */ if (numAllocatedFiles >= MAX_ALLOCATED_FILES || - numAllocatedFiles >= max_safe_fds - 1) - elog(ERROR, "too many private FDs demanded"); + numAllocatedFiles + numAllocatedDirs >= max_safe_fds - 1) + elog(ERROR, "too many private files demanded"); TryAgain: if ((file = fopen(name, mode)) != NULL) @@ -1155,6 +1164,86 @@ FreeFile(FILE *file) return fclose(file); } + +/* + * Routines that want to use (ie, DIR*) should use AllocateDir + * rather than plain opendir(). This lets fd.c deal with freeing FDs if + * necessary to open the directory, and with closing it after an elog. + * When done, call FreeDir rather than closedir. + * + * Ideally this should be the *only* direct call of opendir() in the backend. + */ +DIR * +AllocateDir(const char *dirname) +{ + DIR *dir; + + DO_DB(elog(LOG, "AllocateDir: Allocated %d", numAllocatedDirs)); + + /* + * The test against MAX_ALLOCATED_DIRS prevents us from overflowing + * allocatedDirs[]; the test against max_safe_fds prevents AllocateDir + * from hogging every one of the available FDs, which'd lead to infinite + * looping. + */ + if (numAllocatedDirs >= MAX_ALLOCATED_DIRS || + numAllocatedDirs + numAllocatedFiles >= max_safe_fds - 1) + elog(ERROR, "too many private dirs demanded"); + +TryAgain: + if ((dir = opendir(dirname)) != NULL) + { + allocatedDirs[numAllocatedDirs] = dir; + numAllocatedDirs++; + return dir; + } + + if (errno == EMFILE || errno == ENFILE) + { + int save_errno = errno; + + ereport(LOG, + (errcode(ERRCODE_INSUFFICIENT_RESOURCES), + errmsg("out of file descriptors: %m; release and retry"))); + errno = 0; + if (ReleaseLruFile()) + goto TryAgain; + errno = save_errno; + } + + return NULL; +} + +/* + * Close a directory opened with AllocateDir. + * + * Note we do not check closedir's return value --- it is up to the caller + * to handle close errors. + */ +int +FreeDir(DIR *dir) +{ + int i; + + DO_DB(elog(LOG, "FreeDir: Allocated %d", numAllocatedDirs)); + + /* Remove dir from list of allocated dirs, if it's present */ + for (i = numAllocatedDirs; --i >= 0;) + { + if (allocatedDirs[i] == dir) + { + numAllocatedDirs--; + allocatedDirs[i] = allocatedDirs[numAllocatedDirs]; + break; + } + } + if (i < 0) + elog(WARNING, "dir passed to FreeDir was not obtained from AllocateDir"); + + return closedir(dir); +} + + /* * closeAllVfds * @@ -1211,7 +1300,7 @@ AtProcExit_Files(int code, Datum arg) * 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. + * also clean up "allocated" stdio files and dirs. */ static void CleanupTempFiles(bool isProcExit) @@ -1240,6 +1329,9 @@ CleanupTempFiles(bool isProcExit) while (numAllocatedFiles > 0) FreeFile(allocatedFiles[0]); + + while (numAllocatedDirs > 0) + FreeDir(allocatedDirs[0]); } @@ -1271,7 +1363,7 @@ RemovePgTempFiles(void) * files. */ snprintf(db_path, sizeof(db_path), "%s/base", DataDir); - if ((db_dir = opendir(db_path)) != NULL) + if ((db_dir = AllocateDir(db_path)) != NULL) { while ((db_de = readdir(db_dir)) != NULL) { @@ -1287,7 +1379,7 @@ RemovePgTempFiles(void) "%s/%s/%s", db_path, db_de->d_name, PG_TEMP_FILES_DIR); - if ((temp_dir = opendir(temp_path)) != NULL) + if ((temp_dir = AllocateDir(temp_path)) != NULL) { while ((temp_de = readdir(temp_dir)) != NULL) { @@ -1310,9 +1402,9 @@ RemovePgTempFiles(void) "unexpected file found in temporary-files directory: \"%s\"", rm_path); } - closedir(temp_dir); + FreeDir(temp_dir); } } - closedir(db_dir); + FreeDir(db_dir); } } diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h index feca2b92b1..177925cf3e 100644 --- a/src/include/storage/fd.h +++ b/src/include/storage/fd.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/storage/fd.h,v 1.43 2004/02/23 20:45:59 tgl Exp $ + * $PostgreSQL: pgsql/src/include/storage/fd.h,v 1.44 2004/02/23 23:03:10 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -31,10 +31,16 @@ * use FreeFile, not fclose, to close it. AVOID using stdio for files * that you intend to hold open for any length of time, since there is * no way for them to share kernel file descriptors with other files. + * + * Likewise, use AllocateDir/FreeDir, not opendir/closedir, to allocate + * open directories (DIR*). */ #ifndef FD_H #define FD_H +#include + + /* * FileSeek uses the standard UNIX lseek(2) flags. */ @@ -65,7 +71,11 @@ extern int FileTruncate(File file, long offset); /* Operations that allow use of regular stdio --- USE WITH CAUTION */ extern FILE *AllocateFile(char *name, char *mode); -extern int FreeFile(FILE *); +extern int FreeFile(FILE *file); + +/* Operations to allow use of the library routines */ +extern DIR *AllocateDir(const char *dirname); +extern int FreeDir(DIR *dir); /* If you've really really gotta have a plain kernel FD, use this */ extern int BasicOpenFile(FileName fileName, int fileFlags, int fileMode); diff --git a/src/port/copydir.c b/src/port/copydir.c index 64a8407a7f..7d33d14c82 100644 --- a/src/port/copydir.c +++ b/src/port/copydir.c @@ -11,18 +11,18 @@ * as a service. * * IDENTIFICATION - * $PostgreSQL: pgsql/src/port/copydir.c,v 1.7 2003/11/29 19:52:13 pgsql Exp $ + * $PostgreSQL: pgsql/src/port/copydir.c,v 1.8 2004/02/23 23:03:10 tgl Exp $ * *------------------------------------------------------------------------- */ #include "postgres.h" +#include "storage/fd.h" + #undef mkdir /* no reason to use that macro because we * ignore the 2nd arg */ -#include - /* * copydir: copy a directory (we only need to go one level deep) @@ -47,7 +47,7 @@ copydir(char *fromdir, char *todir) errmsg("could not create directory \"%s\": %m", todir))); return -1; } - xldir = opendir(fromdir); + xldir = AllocateDir(fromdir); if (xldir == NULL) { ereport(WARNING, @@ -65,11 +65,11 @@ copydir(char *fromdir, char *todir) ereport(WARNING, (errcode_for_file_access(), errmsg("could not copy file \"%s\": %m", fromfl))); - closedir(xldir); + FreeDir(xldir); return -1; } } - closedir(xldir); + FreeDir(xldir); return 0; } -- 2.40.0