Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit1f67078

Browse files
committed
Add OpenTransientFile, with automatic cleanup at end-of-xact.
Files opened with BasicOpenFile or PathNameOpenFile are not automaticallycleaned up on error. That puts unnecessary burden on callers that only wantto keep the file open for a short time. There is AllocateFile, but thatreturns a buffered FILE * stream, which in many cases is not the nicest APIto work with. So add function called OpenTransientFile, which returns aunbuffered 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 BasicOpenFile2. 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() callswith OpenTransientFile() that were not leaking, because the code meticulouslyclosed the file on error. That wasn't strictly necessary, but IMHO it's goodfor 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 backpatchlater, after this mechanism has had some more testing in master branch.
1 parent5329942 commit1f67078

File tree

10 files changed

+209
-112
lines changed

10 files changed

+209
-112
lines changed

‎src/backend/access/transam/slru.c

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,7 @@ SlruInternalWritePage(SlruCtl ctl, int slotno, SlruFlush fdata)
531531
inti;
532532

533533
for (i=0;i<fdata->num_files;i++)
534-
close(fdata->fd[i]);
534+
CloseTransientFile(fdata->fd[i]);
535535
}
536536

537537
/* Re-acquire control lock and update page state */
@@ -593,7 +593,7 @@ SlruPhysicalReadPage(SlruCtl ctl, int pageno, int slotno)
593593
* SlruPhysicalWritePage).Hence, if we are InRecovery, allow the case
594594
* where the file doesn't exist, and return zeroes instead.
595595
*/
596-
fd=BasicOpenFile(path,O_RDWR |PG_BINARY,S_IRUSR |S_IWUSR);
596+
fd=OpenTransientFile(path,O_RDWR |PG_BINARY,S_IRUSR |S_IWUSR);
597597
if (fd<0)
598598
{
599599
if (errno!=ENOENT|| !InRecovery)
@@ -614,7 +614,7 @@ SlruPhysicalReadPage(SlruCtl ctl, int pageno, int slotno)
614614
{
615615
slru_errcause=SLRU_SEEK_FAILED;
616616
slru_errno=errno;
617-
close(fd);
617+
CloseTransientFile(fd);
618618
return false;
619619
}
620620

@@ -623,11 +623,11 @@ SlruPhysicalReadPage(SlruCtl ctl, int pageno, int slotno)
623623
{
624624
slru_errcause=SLRU_READ_FAILED;
625625
slru_errno=errno;
626-
close(fd);
626+
CloseTransientFile(fd);
627627
return false;
628628
}
629629

630-
if (close(fd))
630+
if (CloseTransientFile(fd))
631631
{
632632
slru_errcause=SLRU_CLOSE_FAILED;
633633
slru_errno=errno;
@@ -740,8 +740,8 @@ SlruPhysicalWritePage(SlruCtl ctl, int pageno, int slotno, SlruFlush fdata)
740740
* don't use O_EXCL or O_TRUNC or anything like that.
741741
*/
742742
SlruFileName(ctl,path,segno);
743-
fd=BasicOpenFile(path,O_RDWR |O_CREAT |PG_BINARY,
744-
S_IRUSR |S_IWUSR);
743+
fd=OpenTransientFile(path,O_RDWR |O_CREAT |PG_BINARY,
744+
S_IRUSR |S_IWUSR);
745745
if (fd<0)
746746
{
747747
slru_errcause=SLRU_OPEN_FAILED;
@@ -773,7 +773,7 @@ SlruPhysicalWritePage(SlruCtl ctl, int pageno, int slotno, SlruFlush fdata)
773773
slru_errcause=SLRU_SEEK_FAILED;
774774
slru_errno=errno;
775775
if (!fdata)
776-
close(fd);
776+
CloseTransientFile(fd);
777777
return false;
778778
}
779779

@@ -786,7 +786,7 @@ SlruPhysicalWritePage(SlruCtl ctl, int pageno, int slotno, SlruFlush fdata)
786786
slru_errcause=SLRU_WRITE_FAILED;
787787
slru_errno=errno;
788788
if (!fdata)
789-
close(fd);
789+
CloseTransientFile(fd);
790790
return false;
791791
}
792792

@@ -800,11 +800,11 @@ SlruPhysicalWritePage(SlruCtl ctl, int pageno, int slotno, SlruFlush fdata)
800800
{
801801
slru_errcause=SLRU_FSYNC_FAILED;
802802
slru_errno=errno;
803-
close(fd);
803+
CloseTransientFile(fd);
804804
return false;
805805
}
806806

807-
if (close(fd))
807+
if (CloseTransientFile(fd))
808808
{
809809
slru_errcause=SLRU_CLOSE_FAILED;
810810
slru_errno=errno;
@@ -1078,7 +1078,7 @@ SimpleLruFlush(SlruCtl ctl, bool checkpoint)
10781078
ok= false;
10791079
}
10801080

1081-
if (close(fdata.fd[i]))
1081+
if (CloseTransientFile(fdata.fd[i]))
10821082
{
10831083
slru_errcause=SLRU_CLOSE_FAILED;
10841084
slru_errno=errno;

‎src/backend/access/transam/timeline.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,8 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI,
244244
unlink(tmppath);
245245

246246
/* do not use get_sync_bit() here --- want to fsync only at end of fill */
247-
fd=BasicOpenFile(tmppath,O_RDWR |O_CREAT |O_EXCL,
248-
S_IRUSR |S_IWUSR);
247+
fd=OpenTransientFile(tmppath,O_RDWR |O_CREAT |O_EXCL,
248+
S_IRUSR |S_IWUSR);
249249
if (fd<0)
250250
ereport(ERROR,
251251
(errcode_for_file_access(),
@@ -262,7 +262,7 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI,
262262
else
263263
TLHistoryFilePath(path,parentTLI);
264264

265-
srcfd=BasicOpenFile(path,O_RDONLY,0);
265+
srcfd=OpenTransientFile(path,O_RDONLY,0);
266266
if (srcfd<0)
267267
{
268268
if (errno!=ENOENT)
@@ -304,7 +304,7 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI,
304304
errmsg("could not write to file \"%s\": %m",tmppath)));
305305
}
306306
}
307-
close(srcfd);
307+
CloseTransientFile(srcfd);
308308
}
309309

310310
/*
@@ -345,7 +345,7 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI,
345345
(errcode_for_file_access(),
346346
errmsg("could not fsync file \"%s\": %m",tmppath)));
347347

348-
if (close(fd))
348+
if (CloseTransientFile(fd))
349349
ereport(ERROR,
350350
(errcode_for_file_access(),
351351
errmsg("could not close file \"%s\": %m",tmppath)));

‎src/backend/access/transam/twophase.c

Lines changed: 24 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -970,17 +970,12 @@ EndPrepare(GlobalTransaction gxact)
970970

971971
/*
972972
* Create the 2PC state file.
973-
*
974-
* Note: because we use BasicOpenFile(), we are responsible for ensuring
975-
* the FD gets closed in any error exit path. Once we get into the
976-
* critical section, though, it doesn't matter since any failure causes
977-
* PANIC anyway.
978973
*/
979974
TwoPhaseFilePath(path,xid);
980975

981-
fd=BasicOpenFile(path,
982-
O_CREAT |O_EXCL |O_WRONLY |PG_BINARY,
983-
S_IRUSR |S_IWUSR);
976+
fd=OpenTransientFile(path,
977+
O_CREAT |O_EXCL |O_WRONLY |PG_BINARY,
978+
S_IRUSR |S_IWUSR);
984979
if (fd<0)
985980
ereport(ERROR,
986981
(errcode_for_file_access(),
@@ -995,7 +990,7 @@ EndPrepare(GlobalTransaction gxact)
995990
COMP_CRC32(statefile_crc,record->data,record->len);
996991
if ((write(fd,record->data,record->len))!=record->len)
997992
{
998-
close(fd);
993+
CloseTransientFile(fd);
999994
ereport(ERROR,
1000995
(errcode_for_file_access(),
1001996
errmsg("could not write two-phase state file: %m")));
@@ -1012,7 +1007,7 @@ EndPrepare(GlobalTransaction gxact)
10121007

10131008
if ((write(fd,&bogus_crc,sizeof(pg_crc32)))!=sizeof(pg_crc32))
10141009
{
1015-
close(fd);
1010+
CloseTransientFile(fd);
10161011
ereport(ERROR,
10171012
(errcode_for_file_access(),
10181013
errmsg("could not write two-phase state file: %m")));
@@ -1021,7 +1016,7 @@ EndPrepare(GlobalTransaction gxact)
10211016
/* Back up to prepare for rewriting the CRC */
10221017
if (lseek(fd,-((off_t)sizeof(pg_crc32)),SEEK_CUR)<0)
10231018
{
1024-
close(fd);
1019+
CloseTransientFile(fd);
10251020
ereport(ERROR,
10261021
(errcode_for_file_access(),
10271022
errmsg("could not seek in two-phase state file: %m")));
@@ -1061,13 +1056,13 @@ EndPrepare(GlobalTransaction gxact)
10611056
/* write correct CRC and close file */
10621057
if ((write(fd,&statefile_crc,sizeof(pg_crc32)))!=sizeof(pg_crc32))
10631058
{
1064-
close(fd);
1059+
CloseTransientFile(fd);
10651060
ereport(ERROR,
10661061
(errcode_for_file_access(),
10671062
errmsg("could not write two-phase state file: %m")));
10681063
}
10691064

1070-
if (close(fd)!=0)
1065+
if (CloseTransientFile(fd)!=0)
10711066
ereport(ERROR,
10721067
(errcode_for_file_access(),
10731068
errmsg("could not close two-phase state file: %m")));
@@ -1144,7 +1139,7 @@ ReadTwoPhaseFile(TransactionId xid, bool give_warnings)
11441139

11451140
TwoPhaseFilePath(path,xid);
11461141

1147-
fd=BasicOpenFile(path,O_RDONLY |PG_BINARY,0);
1142+
fd=OpenTransientFile(path,O_RDONLY |PG_BINARY,0);
11481143
if (fd<0)
11491144
{
11501145
if (give_warnings)
@@ -1163,7 +1158,7 @@ ReadTwoPhaseFile(TransactionId xid, bool give_warnings)
11631158
*/
11641159
if (fstat(fd,&stat))
11651160
{
1166-
close(fd);
1161+
CloseTransientFile(fd);
11671162
if (give_warnings)
11681163
ereport(WARNING,
11691164
(errcode_for_file_access(),
@@ -1177,14 +1172,14 @@ ReadTwoPhaseFile(TransactionId xid, bool give_warnings)
11771172
sizeof(pg_crc32))||
11781173
stat.st_size>MaxAllocSize)
11791174
{
1180-
close(fd);
1175+
CloseTransientFile(fd);
11811176
returnNULL;
11821177
}
11831178

11841179
crc_offset=stat.st_size-sizeof(pg_crc32);
11851180
if (crc_offset!=MAXALIGN(crc_offset))
11861181
{
1187-
close(fd);
1182+
CloseTransientFile(fd);
11881183
returnNULL;
11891184
}
11901185

@@ -1195,7 +1190,7 @@ ReadTwoPhaseFile(TransactionId xid, bool give_warnings)
11951190

11961191
if (read(fd,buf,stat.st_size)!=stat.st_size)
11971192
{
1198-
close(fd);
1193+
CloseTransientFile(fd);
11991194
if (give_warnings)
12001195
ereport(WARNING,
12011196
(errcode_for_file_access(),
@@ -1205,7 +1200,7 @@ ReadTwoPhaseFile(TransactionId xid, bool give_warnings)
12051200
returnNULL;
12061201
}
12071202

1208-
close(fd);
1203+
CloseTransientFile(fd);
12091204

12101205
hdr= (TwoPhaseFileHeader*)buf;
12111206
if (hdr->magic!=TWOPHASE_MAGIC||hdr->total_len!=stat.st_size)
@@ -1469,9 +1464,9 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int len)
14691464

14701465
TwoPhaseFilePath(path,xid);
14711466

1472-
fd=BasicOpenFile(path,
1473-
O_CREAT |O_TRUNC |O_WRONLY |PG_BINARY,
1474-
S_IRUSR |S_IWUSR);
1467+
fd=OpenTransientFile(path,
1468+
O_CREAT |O_TRUNC |O_WRONLY |PG_BINARY,
1469+
S_IRUSR |S_IWUSR);
14751470
if (fd<0)
14761471
ereport(ERROR,
14771472
(errcode_for_file_access(),
@@ -1481,14 +1476,14 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int len)
14811476
/* Write content and CRC */
14821477
if (write(fd,content,len)!=len)
14831478
{
1484-
close(fd);
1479+
CloseTransientFile(fd);
14851480
ereport(ERROR,
14861481
(errcode_for_file_access(),
14871482
errmsg("could not write two-phase state file: %m")));
14881483
}
14891484
if (write(fd,&statefile_crc,sizeof(pg_crc32))!=sizeof(pg_crc32))
14901485
{
1491-
close(fd);
1486+
CloseTransientFile(fd);
14921487
ereport(ERROR,
14931488
(errcode_for_file_access(),
14941489
errmsg("could not write two-phase state file: %m")));
@@ -1500,13 +1495,13 @@ RecreateTwoPhaseFile(TransactionId xid, void *content, int len)
15001495
*/
15011496
if (pg_fsync(fd)!=0)
15021497
{
1503-
close(fd);
1498+
CloseTransientFile(fd);
15041499
ereport(ERROR,
15051500
(errcode_for_file_access(),
15061501
errmsg("could not fsync two-phase state file: %m")));
15071502
}
15081503

1509-
if (close(fd)!=0)
1504+
if (CloseTransientFile(fd)!=0)
15101505
ereport(ERROR,
15111506
(errcode_for_file_access(),
15121507
errmsg("could not close two-phase state file: %m")));
@@ -1577,7 +1572,7 @@ CheckPointTwoPhase(XLogRecPtr redo_horizon)
15771572

15781573
TwoPhaseFilePath(path,xid);
15791574

1580-
fd=BasicOpenFile(path,O_RDWR |PG_BINARY,0);
1575+
fd=OpenTransientFile(path,O_RDWR |PG_BINARY,0);
15811576
if (fd<0)
15821577
{
15831578
if (errno==ENOENT)
@@ -1596,14 +1591,14 @@ CheckPointTwoPhase(XLogRecPtr redo_horizon)
15961591

15971592
if (pg_fsync(fd)!=0)
15981593
{
1599-
close(fd);
1594+
CloseTransientFile(fd);
16001595
ereport(ERROR,
16011596
(errcode_for_file_access(),
16021597
errmsg("could not fsync two-phase state file \"%s\": %m",
16031598
path)));
16041599
}
16051600

1606-
if (close(fd)!=0)
1601+
if (CloseTransientFile(fd)!=0)
16071602
ereport(ERROR,
16081603
(errcode_for_file_access(),
16091604
errmsg("could not close two-phase state file \"%s\": %m",

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp