1
1
Fork 0
mirror of https://github.com/NixOS/nix.git synced 2025-11-14 14:32:42 +01:00

libutil: Make PosixSourceAccessor update mtime only when needed

Typically PosixSourceAccessor can be used from multiple threads,
but mtime is not updated atomically (i.e. with compare_exchange_weak),
so mtime gets raced. It's only needed in dumpPathAndGetMtime and mtime
tracking can be gated behind that.

Also start using getLastModified interface instead of dynamic casts.
This commit is contained in:
Sergei Zimmerman 2025-11-13 23:54:14 +03:00
parent 3645671570
commit e95503cf9a
No known key found for this signature in database
3 changed files with 27 additions and 11 deletions

View file

@ -103,9 +103,9 @@ void SourceAccessor::dumpPath(const CanonPath & path, Sink & sink, PathFilter &
time_t dumpPathAndGetMtime(const Path & path, Sink & sink, PathFilter & filter) time_t dumpPathAndGetMtime(const Path & path, Sink & sink, PathFilter & filter)
{ {
auto path2 = PosixSourceAccessor::createAtRoot(path); auto path2 = PosixSourceAccessor::createAtRoot(path, /*trackLastModified=*/true);
path2.dumpPath(sink, filter); path2.dumpPath(sink, filter);
return path2.accessor.dynamic_pointer_cast<PosixSourceAccessor>()->mtime; return path2.accessor->getLastModified().value();
} }
void dumpPath(const Path & path, Sink & sink, PathFilter & filter) void dumpPath(const Path & path, Sink & sink, PathFilter & filter)

View file

@ -9,7 +9,7 @@ struct SourcePath;
/** /**
* A source accessor that uses the Unix filesystem. * A source accessor that uses the Unix filesystem.
*/ */
struct PosixSourceAccessor : virtual SourceAccessor class PosixSourceAccessor : virtual public SourceAccessor
{ {
/** /**
* Optional root path to prefix all operations into the native file * Optional root path to prefix all operations into the native file
@ -18,8 +18,12 @@ struct PosixSourceAccessor : virtual SourceAccessor
*/ */
const std::filesystem::path root; const std::filesystem::path root;
const bool trackLastModified = false;
public:
PosixSourceAccessor(); PosixSourceAccessor();
PosixSourceAccessor(std::filesystem::path && root); PosixSourceAccessor(std::filesystem::path && root, bool trackLastModified = false);
/** /**
* The most recent mtime seen by lstat(). This is a hack to * The most recent mtime seen by lstat(). This is a hack to
@ -43,6 +47,9 @@ struct PosixSourceAccessor : virtual SourceAccessor
* Create a `PosixSourceAccessor` and `SourcePath` corresponding to * Create a `PosixSourceAccessor` and `SourcePath` corresponding to
* some native path. * some native path.
* *
* @param Whether the accessor should return a non-null getLastModified.
* When true the accessor must be used only by a single thread.
*
* The `PosixSourceAccessor` is rooted as far up the tree as * The `PosixSourceAccessor` is rooted as far up the tree as
* possible, (e.g. on Windows it could scoped to a drive like * possible, (e.g. on Windows it could scoped to a drive like
* `C:\`). This allows more `..` parent accessing to work. * `C:\`). This allows more `..` parent accessing to work.
@ -64,7 +71,12 @@ struct PosixSourceAccessor : virtual SourceAccessor
* and * and
* [`std::filesystem::path::relative_path`](https://en.cppreference.com/w/cpp/filesystem/path/relative_path). * [`std::filesystem::path::relative_path`](https://en.cppreference.com/w/cpp/filesystem/path/relative_path).
*/ */
static SourcePath createAtRoot(const std::filesystem::path & path); static SourcePath createAtRoot(const std::filesystem::path & path, bool trackLastModified = false);
std::optional<std::time_t> getLastModified() override
{
return trackLastModified ? std::optional{mtime} : std::nullopt;
}
private: private:

View file

@ -7,8 +7,9 @@
namespace nix { namespace nix {
PosixSourceAccessor::PosixSourceAccessor(std::filesystem::path && argRoot) PosixSourceAccessor::PosixSourceAccessor(std::filesystem::path && argRoot, bool trackLastModified)
: root(std::move(argRoot)) : root(std::move(argRoot))
, trackLastModified(trackLastModified)
{ {
assert(root.empty() || root.is_absolute()); assert(root.empty() || root.is_absolute());
displayPrefix = root.string(); displayPrefix = root.string();
@ -19,11 +20,11 @@ PosixSourceAccessor::PosixSourceAccessor()
{ {
} }
SourcePath PosixSourceAccessor::createAtRoot(const std::filesystem::path & path) SourcePath PosixSourceAccessor::createAtRoot(const std::filesystem::path & path, bool trackLastModified)
{ {
std::filesystem::path path2 = absPath(path); std::filesystem::path path2 = absPath(path);
return { return {
make_ref<PosixSourceAccessor>(path2.root_path()), make_ref<PosixSourceAccessor>(path2.root_path(), trackLastModified),
CanonPath{path2.relative_path().string()}, CanonPath{path2.relative_path().string()},
}; };
} }
@ -114,9 +115,12 @@ std::optional<SourceAccessor::Stat> PosixSourceAccessor::maybeLstat(const CanonP
auto st = cachedLstat(path); auto st = cachedLstat(path);
if (!st) if (!st)
return std::nullopt; return std::nullopt;
// This makes the accessor thread-unsafe, but we only seem to use the actual value in a single threaded context in
// `src/libfetchers/path.cc`. /* The contract is that trackLastModified implies that the caller uses the accessor
from a single thread. Thus this is not a CAS loop. */
if (trackLastModified)
mtime = std::max(mtime, st->st_mtime); mtime = std::max(mtime, st->st_mtime);
return Stat{ return Stat{
.type = S_ISREG(st->st_mode) ? tRegular .type = S_ISREG(st->st_mode) ? tRegular
: S_ISDIR(st->st_mode) ? tDirectory : S_ISDIR(st->st_mode) ? tDirectory