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

Commit82a5649

Browse files
committed
Tighten use of OpenTransientFile and CloseTransientFile
This fixes two sets of issues related to the use of transient files inthe backend:1) OpenTransientFile() has been used in some code paths with read-writeflags while read-only is sufficient, so switch those calls to beread-only where necessary. These have been reported by Joe Conway.2) When opening transient files, it is up to the caller to close thefile descriptors opened. In error code paths, CloseTransientFile() getscalled to clean up things before issuing an error. However in normalexit paths, a lot of callers of CloseTransientFile() never actuallyreported errors, which could leave a file descriptor open withoutknowing about it. This is an issue I complained about a couple oftimes, but never had the courage to write and submit a patch, so here wego.Note that one frontend code path is impacted by this commit so as anerror is issued when fetching control file data, making backend andfrontend to be treated consistently.Reported-by: Joe Conway, Michael PaquierAuthor: Michael PaquierReviewed-by: Álvaro Herrera, Georgios Kokolatos, Joe ConwayDiscussion:https://postgr.es/m/20190301023338.GD1348@paquier.xyzDiscussion:https://postgr.es/m/c49b69ec-e2f7-ff33-4f17-0eaa4f2cef27@joeconway.com
1 parent2e616de commit82a5649

File tree

17 files changed

+134
-30
lines changed

17 files changed

+134
-30
lines changed

‎contrib/pg_stat_statements/pg_stat_statements.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1991,7 +1991,10 @@ qtext_load_file(Size *buffer_size)
19911991
returnNULL;
19921992
}
19931993

1994-
CloseTransientFile(fd);
1994+
if (CloseTransientFile(fd))
1995+
ereport(LOG,
1996+
(errcode_for_file_access(),
1997+
errmsg("could not close file \"%s\": %m",PGSS_TEXT_FILE)));
19951998

19961999
*buffer_size=stat.st_size;
19972000
returnbuf;

‎src/backend/access/heap/rewriteheap.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1202,7 +1202,10 @@ heap_xlog_logical_rewrite(XLogReaderState *r)
12021202
errmsg("could not fsync file \"%s\": %m",path)));
12031203
pgstat_report_wait_end();
12041204

1205-
CloseTransientFile(fd);
1205+
if (CloseTransientFile(fd))
1206+
ereport(ERROR,
1207+
(errcode_for_file_access(),
1208+
errmsg("could not close file \"%s\": %m",path)));
12061209
}
12071210

12081211
/* ---
@@ -1300,7 +1303,11 @@ CheckPointLogicalRewriteHeap(void)
13001303
(errcode_for_file_access(),
13011304
errmsg("could not fsync file \"%s\": %m",path)));
13021305
pgstat_report_wait_end();
1303-
CloseTransientFile(fd);
1306+
1307+
if (CloseTransientFile(fd))
1308+
ereport(ERROR,
1309+
(errcode_for_file_access(),
1310+
errmsg("could not close file \"%s\": %m",path)));
13041311
}
13051312
}
13061313
FreeDir(mappings_dir);

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,7 @@ SimpleLruDoesPhysicalPageExist(SlruCtl ctl, int pageno)
599599

600600
SlruFileName(ctl,path,segno);
601601

602-
fd=OpenTransientFile(path,O_RDWR |PG_BINARY);
602+
fd=OpenTransientFile(path,O_RDONLY |PG_BINARY);
603603
if (fd<0)
604604
{
605605
/* expected: file doesn't exist */
@@ -621,7 +621,13 @@ SimpleLruDoesPhysicalPageExist(SlruCtl ctl, int pageno)
621621

622622
result=endpos >= (off_t) (offset+BLCKSZ);
623623

624-
CloseTransientFile(fd);
624+
if (CloseTransientFile(fd))
625+
{
626+
slru_errcause=SLRU_CLOSE_FAILED;
627+
slru_errno=errno;
628+
return false;
629+
}
630+
625631
returnresult;
626632
}
627633

@@ -654,7 +660,7 @@ SlruPhysicalReadPage(SlruCtl ctl, int pageno, int slotno)
654660
* SlruPhysicalWritePage). Hence, if we are InRecovery, allow the case
655661
* where the file doesn't exist, and return zeroes instead.
656662
*/
657-
fd=OpenTransientFile(path,O_RDWR |PG_BINARY);
663+
fd=OpenTransientFile(path,O_RDONLY |PG_BINARY);
658664
if (fd<0)
659665
{
660666
if (errno!=ENOENT|| !InRecovery)

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,11 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI,
370370
}
371371
pgstat_report_wait_end();
372372
}
373-
CloseTransientFile(srcfd);
373+
374+
if (CloseTransientFile(srcfd))
375+
ereport(ERROR,
376+
(errcode_for_file_access(),
377+
errmsg("could not close file \"%s\": %m",path)));
374378
}
375379

376380
/*
@@ -416,7 +420,6 @@ writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI,
416420
(errcode_for_file_access(),
417421
errmsg("could not close file \"%s\": %m",tmppath)));
418422

419-
420423
/*
421424
* Now move the completed history file into place with its final name.
422425
*/
@@ -495,7 +498,6 @@ writeTimeLineHistoryFile(TimeLineID tli, char *content, int size)
495498
(errcode_for_file_access(),
496499
errmsg("could not close file \"%s\": %m",tmppath)));
497500

498-
499501
/*
500502
* Now move the completed history file into place with its final name.
501503
*/

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1297,7 +1297,11 @@ ReadTwoPhaseFile(TransactionId xid, bool missing_ok)
12971297
}
12981298

12991299
pgstat_report_wait_end();
1300-
CloseTransientFile(fd);
1300+
1301+
if (CloseTransientFile(fd))
1302+
ereport(ERROR,
1303+
(errcode_for_file_access(),
1304+
errmsg("could not close file \"%s\": %m",path)));
13011305

13021306
hdr= (TwoPhaseFileHeader*)buf;
13031307
if (hdr->magic!=TWOPHASE_MAGIC)

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3469,7 +3469,10 @@ XLogFileCopy(XLogSegNo destsegno, TimeLineID srcTLI, XLogSegNo srcsegno,
34693469
(errcode_for_file_access(),
34703470
errmsg("could not close file \"%s\": %m",tmppath)));
34713471

3472-
CloseTransientFile(srcfd);
3472+
if (CloseTransientFile(srcfd))
3473+
ereport(ERROR,
3474+
(errcode_for_file_access(),
3475+
errmsg("could not close file \"%s\": %m",path)));
34733476

34743477
/*
34753478
* Now move the segment into place with its final name.

‎src/backend/libpq/be-fsstubs.c

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,12 @@ lo_import_internal(text *filename, Oid lobjOid)
455455
fnamebuf)));
456456

457457
inv_close(lobj);
458-
CloseTransientFile(fd);
458+
459+
if (CloseTransientFile(fd))
460+
ereport(ERROR,
461+
(errcode_for_file_access(),
462+
errmsg("could not close file \"%s\": %m",
463+
fnamebuf)));
459464

460465
returnoid;
461466
}
@@ -524,7 +529,12 @@ be_lo_export(PG_FUNCTION_ARGS)
524529
fnamebuf)));
525530
}
526531

527-
CloseTransientFile(fd);
532+
if (CloseTransientFile(fd))
533+
ereport(ERROR,
534+
(errcode_for_file_access(),
535+
errmsg("could not close file \"%s\": %m",
536+
fnamebuf)));
537+
528538
inv_close(lobj);
529539

530540
PG_RETURN_INT32(1);

‎src/backend/replication/logical/origin.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -658,7 +658,11 @@ CheckPointReplicationOrigin(void)
658658
tmppath)));
659659
}
660660

661-
CloseTransientFile(tmpfd);
661+
if (CloseTransientFile(tmpfd))
662+
ereport(PANIC,
663+
(errcode_for_file_access(),
664+
errmsg("could not close file \"%s\": %m",
665+
tmppath)));
662666

663667
/* fsync, rename to permanent file, fsync file and directory */
664668
durable_rename(tmppath,path,PANIC);
@@ -793,7 +797,11 @@ StartupReplicationOrigin(void)
793797
errmsg("replication slot checkpoint has wrong checksum %u, expected %u",
794798
crc,file_crc)));
795799

796-
CloseTransientFile(fd);
800+
if (CloseTransientFile(fd))
801+
ereport(PANIC,
802+
(errcode_for_file_access(),
803+
errmsg("could not close file \"%s\": %m",
804+
path)));
797805
}
798806

799807
void

‎src/backend/replication/logical/reorderbuffer.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3360,7 +3360,10 @@ ApplyLogicalMappingFile(HTAB *tuplecid_data, Oid relid, const char *fname)
33603360
}
33613361
}
33623362

3363-
CloseTransientFile(fd);
3363+
if (CloseTransientFile(fd))
3364+
ereport(ERROR,
3365+
(errcode_for_file_access(),
3366+
errmsg("could not close file \"%s\": %m",path)));
33643367
}
33653368

33663369

‎src/backend/replication/logical/snapbuild.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1651,7 +1651,11 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
16511651
errmsg("could not fsync file \"%s\": %m",tmppath)));
16521652
}
16531653
pgstat_report_wait_end();
1654-
CloseTransientFile(fd);
1654+
1655+
if (CloseTransientFile(fd))
1656+
ereport(ERROR,
1657+
(errcode_for_file_access(),
1658+
errmsg("could not close file \"%s\": %m",tmppath)));
16551659

16561660
fsync_fname("pg_logical/snapshots", true);
16571661

@@ -1846,7 +1850,10 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
18461850
}
18471851
COMP_CRC32C(checksum,ondisk.builder.committed.xip,sz);
18481852

1849-
CloseTransientFile(fd);
1853+
if (CloseTransientFile(fd))
1854+
ereport(ERROR,
1855+
(errcode_for_file_access(),
1856+
errmsg("could not close file \"%s\": %m",path)));
18501857

18511858
FIN_CRC32C(checksum);
18521859

‎src/backend/replication/slot.c

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,7 +1315,11 @@ SaveSlotToPath(ReplicationSlot *slot, const char *dir, int elevel)
13151315
}
13161316
pgstat_report_wait_end();
13171317

1318-
CloseTransientFile(fd);
1318+
if (CloseTransientFile(fd))
1319+
ereport(elevel,
1320+
(errcode_for_file_access(),
1321+
errmsg("could not close file \"%s\": %m",
1322+
tmppath)));
13191323

13201324
/* rename to permanent file, fsync file and directory */
13211325
if (rename(tmppath,path)!=0)
@@ -1377,7 +1381,7 @@ RestoreSlotFromDisk(const char *name)
13771381

13781382
elog(DEBUG1,"restoring replication slot from \"%s\"",path);
13791383

1380-
fd=OpenTransientFile(path,O_RDWR |PG_BINARY);
1384+
fd=OpenTransientFile(path,O_RDONLY |PG_BINARY);
13811385

13821386
/*
13831387
* We do not need to handle this as we are rename()ing the directory into
@@ -1477,7 +1481,10 @@ RestoreSlotFromDisk(const char *name)
14771481
path,readBytes, (Size)cp.length)));
14781482
}
14791483

1480-
CloseTransientFile(fd);
1484+
if (CloseTransientFile(fd))
1485+
ereport(PANIC,
1486+
(errcode_for_file_access(),
1487+
errmsg("could not close file \"%s\": %m",path)));
14811488

14821489
/* now verify the CRC */
14831490
INIT_CRC32C(checksum);

‎src/backend/replication/walsender.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,11 @@ SendTimeLineHistory(TimeLineHistoryCmd *cmd)
521521
pq_sendbytes(&buf,rbuf.data,nread);
522522
bytesleft-=nread;
523523
}
524-
CloseTransientFile(fd);
524+
525+
if (CloseTransientFile(fd))
526+
ereport(ERROR,
527+
(errcode_for_file_access(),
528+
errmsg("could not close file \"%s\": %m",path)));
525529

526530
pq_endmessage(&buf);
527531
}

‎src/backend/storage/file/copydir.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,10 @@ copy_file(char *fromfile, char *tofile)
218218
(errcode_for_file_access(),
219219
errmsg("could not close file \"%s\": %m",tofile)));
220220

221-
CloseTransientFile(srcfd);
221+
if (CloseTransientFile(srcfd))
222+
ereport(ERROR,
223+
(errcode_for_file_access(),
224+
errmsg("could not close file \"%s\": %m",fromfile)));
222225

223226
pfree(buffer);
224227
}

‎src/backend/storage/file/fd.c

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,14 @@ durable_rename(const char *oldfile, const char *newfile, int elevel)
646646
errmsg("could not fsync file \"%s\": %m",newfile)));
647647
return-1;
648648
}
649-
CloseTransientFile(fd);
649+
650+
if (CloseTransientFile(fd))
651+
{
652+
ereport(elevel,
653+
(errcode_for_file_access(),
654+
errmsg("could not close file \"%s\": %m",newfile)));
655+
return-1;
656+
}
650657
}
651658

652659
/* Time to do the real deal... */
@@ -3295,7 +3302,10 @@ pre_sync_fname(const char *fname, bool isdir, int elevel)
32953302
*/
32963303
pg_flush_data(fd,0,0);
32973304

3298-
(void)CloseTransientFile(fd);
3305+
if (CloseTransientFile(fd))
3306+
ereport(elevel,
3307+
(errcode_for_file_access(),
3308+
errmsg("could not close file \"%s\": %m",fname)));
32993309
}
33003310

33013311
#endif/* PG_FLUSH_DATA_WORKS */
@@ -3394,7 +3404,13 @@ fsync_fname_ext(const char *fname, bool isdir, bool ignore_perm, int elevel)
33943404
return-1;
33953405
}
33963406

3397-
(void)CloseTransientFile(fd);
3407+
if (CloseTransientFile(fd))
3408+
{
3409+
ereport(elevel,
3410+
(errcode_for_file_access(),
3411+
errmsg("could not close file \"%s\": %m",fname)));
3412+
return-1;
3413+
}
33983414

33993415
return0;
34003416
}

‎src/backend/storage/ipc/dsm_impl.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -916,7 +916,15 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size,
916916
}
917917
*mapped_address=address;
918918
*mapped_size=request_size;
919-
CloseTransientFile(fd);
919+
920+
if (CloseTransientFile(fd))
921+
{
922+
ereport(elevel,
923+
(errcode_for_file_access(),
924+
errmsg("could not close shared memory segment \"%s\": %m",
925+
name)));
926+
return false;
927+
}
920928

921929
return true;
922930
}

‎src/backend/utils/cache/relmapper.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,11 @@ load_relmap_file(bool shared)
747747
}
748748
pgstat_report_wait_end();
749749

750-
CloseTransientFile(fd);
750+
if (CloseTransientFile(fd))
751+
ereport(FATAL,
752+
(errcode_for_file_access(),
753+
errmsg("could not close file \"%s\": %m",
754+
mapfilename)));
751755

752756
/* check for correct magic number, etc */
753757
if (map->magic!=RELMAPPER_FILEMAGIC||

‎src/common/controldata_utils.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,18 @@ get_controlfile(const char *DataDir, const char *progname, bool *crc_ok_p)
100100
}
101101

102102
#ifndefFRONTEND
103-
CloseTransientFile(fd);
103+
if (CloseTransientFile(fd))
104+
ereport(ERROR,
105+
(errcode_for_file_access(),
106+
errmsg("could not close file \"%s\": %m",
107+
ControlFilePath)));
104108
#else
105-
close(fd);
109+
if (close(fd))
110+
{
111+
fprintf(stderr,_("%s: could not close file \"%s\": %s\n"),
112+
progname,ControlFilePath,strerror(errno));
113+
exit(EXIT_FAILURE);
114+
}
106115
#endif
107116

108117
/* Check the CRC. */

0 commit comments

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