Skip to content

Commit cf02779

Browse files
committed
Add logging and error handling for file syncing
Add logging and error handling inside, and outside of FileCommit. Functions such as fsync, fdatasync will return error in case of hardware I/O errors, and ignoring this means it can silently continue through data corruption. (c.f. https://lwn.net/SubscriberLink/752063/12b232ab5039efbe/)
1 parent 8b262eb commit cf02779

File tree

4 files changed

+36
-13
lines changed

4 files changed

+36
-13
lines changed

src/addrdb.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ bool SerializeFileDB(const std::string& prefix, const fs::path& path, const Data
4949

5050
// Serialize
5151
if (!SerializeDB(fileout, data)) return false;
52-
FileCommit(fileout.Get());
52+
if (!FileCommit(fileout.Get()))
53+
return error("%s: Failed to flush file %s", __func__, pathTmp.string());
5354
fileout.fclose();
5455

5556
// replace existing file, if any, with new file

src/util.cpp

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -788,21 +788,37 @@ bool TryCreateDirectories(const fs::path& p)
788788
return false;
789789
}
790790

791-
void FileCommit(FILE *file)
791+
bool FileCommit(FILE *file)
792792
{
793-
fflush(file); // harmless if redundantly called
793+
if (fflush(file) != 0) { // harmless if redundantly called
794+
LogPrintf("%s: fflush failed: %d\n", __func__, errno);
795+
return false;
796+
}
794797
#ifdef WIN32
795798
HANDLE hFile = (HANDLE)_get_osfhandle(_fileno(file));
796-
FlushFileBuffers(hFile);
799+
if (FlushFileBuffers(hFile) == 0) {
800+
LogPrintf("%s: FlushFileBuffers failed: %d\n", __func__, GetLastError());
801+
return false;
802+
}
797803
#else
798804
#if defined(__linux__) || defined(__NetBSD__)
799-
fdatasync(fileno(file));
805+
if (fdatasync(fileno(file)) != 0 && errno != EINVAL) { // Ignore EINVAL for filesystems that don't support sync
806+
LogPrintf("%s: fdatasync failed: %d\n", __func__, errno);
807+
return false;
808+
}
800809
#elif defined(__APPLE__) && defined(F_FULLFSYNC)
801-
fcntl(fileno(file), F_FULLFSYNC, 0);
810+
if (fcntl(fileno(file), F_FULLFSYNC, 0) == -1) { // Manpage says "value other than -1" is returned on success
811+
LogPrintf("%s: fcntl F_FULLFSYNC failed: %d\n", __func__, errno);
812+
return false;
813+
}
802814
#else
803-
fsync(fileno(file));
815+
if (fsync(fileno(file)) != 0 && errno != EINVAL) {
816+
LogPrintf("%s: fsync failed: %d\n", __func__, errno);
817+
return false;
818+
}
804819
#endif
805820
#endif
821+
return true;
806822
}
807823

808824
bool TruncateFile(FILE *file, unsigned int length) {

src/util.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ bool error(const char* fmt, const Args&... args)
7171
}
7272

7373
void PrintExceptionContinue(const std::exception *pex, const char* pszThread);
74-
void FileCommit(FILE *file);
74+
bool FileCommit(FILE *file);
7575
bool TruncateFile(FILE *file, unsigned int length);
7676
int RaiseFileDescriptorLimit(int nMinFD);
7777
void AllocateFileRange(FILE *file, unsigned int offset, unsigned int length);

src/validation.cpp

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1615,22 +1615,27 @@ void static FlushBlockFile(bool fFinalize = false)
16151615
LOCK(cs_LastBlockFile);
16161616

16171617
CDiskBlockPos posOld(nLastBlockFile, 0);
1618+
bool status = true;
16181619

16191620
FILE *fileOld = OpenBlockFile(posOld);
16201621
if (fileOld) {
16211622
if (fFinalize)
1622-
TruncateFile(fileOld, vinfoBlockFile[nLastBlockFile].nSize);
1623-
FileCommit(fileOld);
1623+
status &= TruncateFile(fileOld, vinfoBlockFile[nLastBlockFile].nSize);
1624+
status &= FileCommit(fileOld);
16241625
fclose(fileOld);
16251626
}
16261627

16271628
fileOld = OpenUndoFile(posOld);
16281629
if (fileOld) {
16291630
if (fFinalize)
1630-
TruncateFile(fileOld, vinfoBlockFile[nLastBlockFile].nUndoSize);
1631-
FileCommit(fileOld);
1631+
status &= TruncateFile(fileOld, vinfoBlockFile[nLastBlockFile].nUndoSize);
1632+
status &= FileCommit(fileOld);
16321633
fclose(fileOld);
16331634
}
1635+
1636+
if (!status) {
1637+
AbortNode("Flushing block file to disk failed. This is likely the result of an I/O error.");
1638+
}
16341639
}
16351640

16361641
static bool FindUndoPos(CValidationState &state, int nFile, CDiskBlockPos &pos, unsigned int nAddSize);
@@ -4760,7 +4765,8 @@ bool DumpMempool(void)
47604765
}
47614766

47624767
file << mapDeltas;
4763-
FileCommit(file.Get());
4768+
if (!FileCommit(file.Get()))
4769+
throw std::runtime_error("FileCommit failed");
47644770
file.fclose();
47654771
RenameOver(GetDataDir() / "mempool.dat.new", GetDataDir() / "mempool.dat");
47664772
int64_t last = GetTimeMicros();

0 commit comments

Comments
 (0)