rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix error handling in temp-file deletion with log_temp_files active. 

The original coding in FileClose() reset the file-is-temp flag before
unlinking the file, so that if control came back through due to an error,
it wouldn't try to unlink the file twice.  This was correct when written,
but when the log_temp_files feature was added, the logging action was put
in between those two steps.  An error occurring during the logging action
--- such as a query cancel --- would result in the unlink not getting done
at all, as in recent report from Michael Glaesemann.

To fix this, make sure that we do both the stat and the unlink before doing
anything that could conceivably CHECK_FOR_INTERRUPTS.  There is a judgment
call here, which is which log message to emit first: if you can see only
one, which should it be?  I chose to log unlink failure at the risk of
losing the log_temp_files log message --- after all, if the unlink does
fail, the temp file is still there for you to see.

Back-patch to all versions that have log_temp_files.  The code was OK
before that.
This commit is contained in:
Tom Lane 2010-11-08 22:14:48 -05:00
parent 854ae8c3a6
commit 54428dbe90
1 changed files with 33 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
src/backend/storage/file/fd.c
View File

@ -1032,7 +1032,6 @@ void
FileClose(File file)
{
Vfd *vfdP;
struct stat filestats;
Assert(FileIsValid(file));
@ -1055,15 +1054,36 @@ FileClose(File file)
}
/*
* Delete the file if it was temporary
* Delete the file if it was temporary, and make a log entry if wanted
*/
if (vfdP->fdstate & FD_TEMPORARY)
{
/* reset flag so that die() interrupt won't cause problems */
/*
* If we get an error, as could happen within the ereport/elog calls,
* we'll come right back here during transaction abort. Reset the
* flag to ensure that we can't get into an infinite loop. This code
* is arranged to ensure that the worst-case consequence is failing
* to emit log message(s), not failing to attempt the unlink.
*/
vfdP->fdstate &= ~FD_TEMPORARY;
if (log_temp_files >= 0)
{
if (stat(vfdP->fileName, &filestats) == 0)
struct stat filestats;
int stat_errno;
/* first try the stat() */
if (stat(vfdP->fileName, &filestats))
stat_errno = errno;
else
stat_errno = 0;
/* in any case do the unlink */
if (unlink(vfdP->fileName))
elog(LOG, "could not unlink file \"%s\": %m", vfdP->fileName);
/* and last report the stat results */
if (stat_errno == 0)
{
if ((filestats.st_size / 1024) >= log_temp_files)
ereport(LOG,
@ -1072,10 +1092,17 @@ FileClose(File file)
(unsigned long) filestats.st_size)));
}
else
{
errno = stat_errno;
elog(LOG, "could not stat file \"%s\": %m", vfdP->fileName);
}
}
else
{
/* easy case, just do the unlink */
if (unlink(vfdP->fileName))
elog(LOG, "could not unlink file \"%s\": %m", vfdP->fileName);
}
if (unlink(vfdP->fileName))
elog(LOG, "could not unlink file \"%s\": %m", vfdP->fileName);
}
/* Unregister it from the resource owner */