The rationale for caching the datadir is given as
0 // This can be called during exceptions by LogPrintf(), so we cache the
1 // value so we don't have to do memory allocations after that.
However, since 8c2d695c4a45bdd9378c7970b0fcba6e1efc01f9, the debug log location is actually cached itself in m_file_path.
So remove the now-useless caching.
701@@ -702,23 +702,9 @@ fs::path GetDefaultDataDir()
702 #endif
703 }
704
705-static fs::path g_blocks_path_cache_net_specific;
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
A side-effect of caching was that this code was only run the first call to GetDataDir. Right now, it will try to create the data directory every time:
0 if (fs::create_directories(path)) {
1 // This is the first run, create wallets subdirectory too
2 fs::create_directories(path / "wallets");
3 }
(this not only results in system call spam, but also a change in behavior if, for some reason, the datadir disappears) A simple solution to avoid this would be to add a static bool flag for this.
but also a change in behavior if, for some reason, the datadir disappears
that’s good?
@laanwj wouldn’t it be better to just call create_directories as soon as possible, like it should belong to the startup? Currently is kind of hard to know when directories are actually created.
that’s good?
That definitely isn’t good, if the datadir disappears while running you want the process to die as soon as possible and not mess with creating directories.
@laanwj wouldn’t it be better to just call create_directories as soon as possible, like it should belong to the startup? Currently is kind of hard to know when directories are actually created.
Yes that would be better, however this is very hard to reason about, and review, with the complexity of our startup sequence. Maybe better to leave for a future PR, which is why I suggested the most straightforward solution.
It is indeed weird to write to disk in a method, whose name implies it is a simple getter.
Added a guard to the disk access for now, but this should be improved in the future. (See also “Fix datadir handling” #15864)
fs::create_directories first.