database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-17 17:02:08 +03:00
Code Activity

Add OpenTransientFile, with automatic cleanup at end-of-xact.

Browse Source 
Files opened with BasicOpenFile or PathNameOpenFile are not automatically
cleaned up on error. That puts unnecessary burden on callers that only want
to keep the file open for a short time. There is AllocateFile, but that
returns a buffered FILE * stream, which in many cases is not the nicest API
to work with. So add function called OpenTransientFile, which returns a
unbuffered fd that's cleaned up like the FILE* returned by AllocateFile().

This plugs a few rare fd leaks in error cases:

1. copy_file() - fixed by by using OpenTransientFile instead of BasicOpenFile
2. XLogFileInit() - fixed by adding close() calls to the error cases. Can't
   use OpenTransientFile here because the fd is supposed to persist over
   transaction boundaries.
3. lo_import/lo_export - fixed by using OpenTransientFile instead of
   PathNameOpenFile.

In addition to plugging those leaks, this replaces many BasicOpenFile() calls
with OpenTransientFile() that were not leaking, because the code meticulously
closed the file on error. That wasn't strictly necessary, but IMHO it's good
for robustness.

The same leaks exist in older versions, but given the rarity of the issues,
I'm not backpatching this. Not yet, anyway - it might be good to backpatch
later, after this mechanism has had some more testing in master branch.
This commit is contained in:
Heikki Linnakangas
2012-11-27 10:25:50 +02:00
parent 532994299e
commit 1f67078ea3
10 changed files with 209 additions and 112 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
src/backend/utils/cache/relmapper.c vendored
View File

@ -588,7 +588,8 @@ load_relmap_file(bool shared)
}
/* Read data ... */
fd = BasicOpenFile(mapfilename, O_RDONLY | PG_BINARY, S_IRUSR | S_IWUSR);
fd = OpenTransientFile(mapfilename,
O_RDONLY | PG_BINARY, S_IRUSR | S_IWUSR);
if (fd < 0)
ereport(FATAL,
(errcode_for_file_access(),
@ -608,7 +609,7 @@ load_relmap_file(bool shared)
errmsg("could not read relation mapping file \"%s\": %m",
mapfilename)));
close(fd);
CloseTransientFile(fd);
/* check for correct magic number, etc */
if (map->magic != RELMAPPER_FILEMAGIC ||
@ -672,12 +673,6 @@ write_relmap_file(bool shared, RelMapFile *newmap,
/*
* Open the target file. We prefer to do this before entering the
* critical section, so that an open() failure need not force PANIC.
*
* Note: since we use BasicOpenFile, we are nominally responsible for
* ensuring the fd is closed on error. In practice, this isn't important
* because either an error happens inside the critical section, or we are
* in bootstrap or WAL replay; so an error past this point is always fatal
* anyway.
*/
if (shared)
{
@ -692,9 +687,9 @@ write_relmap_file(bool shared, RelMapFile *newmap,
realmap = &local_map;
}
fd = BasicOpenFile(mapfilename,
O_WRONLY | O_CREAT | PG_BINARY,
S_IRUSR | S_IWUSR);
fd = OpenTransientFile(mapfilename,
O_WRONLY | O_CREAT | PG_BINARY,
S_IRUSR | S_IWUSR);
if (fd < 0)
ereport(ERROR,
(errcode_for_file_access(),
@ -753,7 +748,7 @@ write_relmap_file(bool shared, RelMapFile *newmap,
errmsg("could not fsync relation mapping file \"%s\": %m",
mapfilename)));
if (close(fd))
if (CloseTransientFile(fd))
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not close relation mapping file \"%s\": %m",