Skip to content

Commit 86a8b35

Browse files
committed
Merge #14501: Fix possible data race when committing block files
ef71229 util: Check for file being NULL in DirectoryCommit (Luke Dashjr) 4574904 Fix possible data race when committing block files (Evan Klitzke) 220bb16 util: Introduce DirectoryCommit commit function to sync a directory (Evan Klitzke) ce5cbae util.h: Document FileCommit function (Evan Klitzke) 844d650 util: Prefer Mac-specific F_FULLSYNC over fdatasync in FileCommit (Evan Klitzke) f6cec0b util: Refactor FileCommit from an #if sequence nested in #else, to a sequence of #elif (Evan Klitzke) Pull request description: Reviving #12696 ACKs for top commit: laanwj: Code review ACK ef71229 Tree-SHA512: 07d650990ef4c18d645dee3f9a199a940683ad17557d79d93979a76c4e710d8d70e6eae01d1a5991494a24a7654eb7db868be0c34a31e70b2509945d95bc9cce
2 parents d7e2401 + ef71229 commit 86a8b35

File tree

3 files changed

+31
-9
lines changed

3 files changed

+31
-9
lines changed

src/flatfile.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ bool FlatFileSeq::Flush(const FlatFilePos& pos, bool finalize)
9292
fclose(file);
9393
return error("%s: failed to commit file %d", __func__, pos.nFile);
9494
}
95+
DirectoryCommit(m_dir);
9596

9697
fclose(file);
9798
return true;

src/util/system.cpp

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1047,27 +1047,36 @@ bool FileCommit(FILE *file)
10471047
LogPrintf("%s: FlushFileBuffers failed: %d\n", __func__, GetLastError());
10481048
return false;
10491049
}
1050-
#else
1051-
#if HAVE_FDATASYNC
1052-
if (fdatasync(fileno(file)) != 0 && errno != EINVAL) { // Ignore EINVAL for filesystems that don't support sync
1053-
LogPrintf("%s: fdatasync failed: %d\n", __func__, errno);
1054-
return false;
1055-
}
1056-
#elif defined(MAC_OSX) && defined(F_FULLFSYNC)
1050+
#elif defined(MAC_OSX) && defined(F_FULLFSYNC)
10571051
if (fcntl(fileno(file), F_FULLFSYNC, 0) == -1) { // Manpage says "value other than -1" is returned on success
10581052
LogPrintf("%s: fcntl F_FULLFSYNC failed: %d\n", __func__, errno);
10591053
return false;
10601054
}
1061-
#else
1055+
#elif HAVE_FDATASYNC
1056+
if (fdatasync(fileno(file)) != 0 && errno != EINVAL) { // Ignore EINVAL for filesystems that don't support sync
1057+
LogPrintf("%s: fdatasync failed: %d\n", __func__, errno);
1058+
return false;
1059+
}
1060+
#else
10621061
if (fsync(fileno(file)) != 0 && errno != EINVAL) {
10631062
LogPrintf("%s: fsync failed: %d\n", __func__, errno);
10641063
return false;
10651064
}
1066-
#endif
10671065
#endif
10681066
return true;
10691067
}
10701068

1069+
void DirectoryCommit(const fs::path &dirname)
1070+
{
1071+
#ifndef WIN32
1072+
FILE* file = fsbridge::fopen(dirname, "r");
1073+
if (file) {
1074+
fsync(fileno(file));
1075+
fclose(file);
1076+
}
1077+
#endif
1078+
}
1079+
10711080
bool TruncateFile(FILE *file, unsigned int length) {
10721081
#if defined(WIN32)
10731082
return _chsize(_fileno(file), length) == 0;

src/util/system.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,19 @@ bool error(const char* fmt, const Args&... args)
5656
}
5757

5858
void PrintExceptionContinue(const std::exception *pex, const char* pszThread);
59+
60+
/**
61+
* Ensure file contents are fully committed to disk, using a platform-specific
62+
* feature analogous to fsync().
63+
*/
5964
bool FileCommit(FILE *file);
65+
66+
/**
67+
* Sync directory contents. This is required on some environments to ensure that
68+
* newly created files are committed to disk.
69+
*/
70+
void DirectoryCommit(const fs::path &dirname);
71+
6072
bool TruncateFile(FILE *file, unsigned int length);
6173
int RaiseFileDescriptorLimit(int nMinFD);
6274
void AllocateFileRange(FILE *file, unsigned int offset, unsigned int length);

0 commit comments

Comments
 (0)