diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index 34091823f8a1..d99279146580 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -177,7 +177,8 @@ static DownloadTarballResult downloadTarball_( the entire file to disk so libarchive can access it in random-access mode. */ auto [fdTemp, path] = createTempFile("nix-zipfile"); - cleanupTemp.reset(path); + cleanupTemp.cancel(); + cleanupTemp = {path}; debug("downloading '%s' into '%s'...", url, path); { FdSink sink(fdTemp.get()); diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index c1ad56dd587a..ffc1b76d3ab3 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -465,33 +465,31 @@ AutoDelete::AutoDelete(const std::filesystem::path & p, bool recursive) this->recursive = recursive; } +void AutoDelete::deletePath() +{ + if (del) { + if (recursive) + nix::deletePath(_path); + else + std::filesystem::remove(_path); + cancel(); + } +} + AutoDelete::~AutoDelete() { try { - if (del) { - if (recursive) - deletePath(_path); - else { - std::filesystem::remove(_path); - } - } + deletePath(); } catch (...) { ignoreExceptionInDestructor(); } } -void AutoDelete::cancel() +void AutoDelete::cancel() noexcept { del = false; } -void AutoDelete::reset(const std::filesystem::path & p, bool recursive) -{ - _path = p; - this->recursive = recursive; - del = true; -} - ////////////////////////////////////////////////////////////////////// #ifdef __FreeBSD__ @@ -500,7 +498,7 @@ AutoUnmount::AutoUnmount() { } -AutoUnmount::AutoUnmount(Path & p) +AutoUnmount::AutoUnmount(const std::filesystem::path & p) : path(p) , del(true) { @@ -509,20 +507,26 @@ AutoUnmount::AutoUnmount(Path & p) AutoUnmount::~AutoUnmount() { try { - if (del) { - if (unmount(path.c_str(), 0) < 0) { - throw SysError("Failed to unmount path %1%", path); - } - } + unmount(); } catch (...) { ignoreExceptionInDestructor(); } } -void AutoUnmount::cancel() +void AutoUnmount::cancel() noexcept { del = false; } + +void AutoUnmount::unmount() +{ + if (del) { + if (::unmount(path.c_str(), 0) < 0) { + throw SysError("Failed to unmount path %1%", PathFmt(path)); + } + } + cancel(); +} #endif ////////////////////////////////////////////////////////////////////// diff --git a/src/libutil/freebsd/freebsd-jail.cc b/src/libutil/freebsd/freebsd-jail.cc index 90fbe0cd62e5..7a35c0b5e7c7 100644 --- a/src/libutil/freebsd/freebsd-jail.cc +++ b/src/libutil/freebsd/freebsd-jail.cc @@ -11,39 +11,35 @@ namespace nix { -AutoRemoveJail::AutoRemoveJail() - : del{false} -{ -} +AutoRemoveJail::AutoRemoveJail() = default; AutoRemoveJail::AutoRemoveJail(int jid) : jid(jid) - , del(true) { } -AutoRemoveJail::~AutoRemoveJail() +void AutoRemoveJail::remove() { - try { - if (del) { - if (jail_remove(jid) < 0) { - throw SysError("Failed to remove jail %1%", jid); - } + if (jid != INVALID_JAIL) { + if (jail_remove(jid) < 0) { + throw SysError("Failed to remove jail %1%", jid); } - } catch (...) { - ignoreExceptionInDestructor(); } + cancel(); } -void AutoRemoveJail::cancel() +AutoRemoveJail::~AutoRemoveJail() { - del = false; + try { + remove(); + } catch (...) { + ignoreExceptionInDestructor(); + } } -void AutoRemoveJail::reset(int j) +void AutoRemoveJail::cancel() noexcept { - del = true; - jid = j; + jid = INVALID_JAIL; } ////////////////////////////////////////////////////////////////////// diff --git a/src/libutil/freebsd/include/nix/util/freebsd-jail.hh b/src/libutil/freebsd/include/nix/util/freebsd-jail.hh index cfab3c2a6440..b4b1cc75a067 100644 --- a/src/libutil/freebsd/include/nix/util/freebsd-jail.hh +++ b/src/libutil/freebsd/include/nix/util/freebsd-jail.hh @@ -7,32 +7,51 @@ namespace nix { class AutoRemoveJail { - int jid; - bool del; + static constexpr int INVALID_JAIL = -1; + int jid = INVALID_JAIL; public: + AutoRemoveJail() = default; AutoRemoveJail(int jid); AutoRemoveJail(const AutoRemoveJail &) = delete; AutoRemoveJail & operator=(const AutoRemoveJail &) = delete; AutoRemoveJail(AutoRemoveJail && other) noexcept : jid(other.jid) - , del(other.del) { other.cancel(); } AutoRemoveJail & operator=(AutoRemoveJail && other) noexcept { - jid = other.jid; - del = other.del; - other.cancel(); + swap(*this, other); return *this; } - AutoRemoveJail(); + friend void swap(AutoRemoveJail & lhs, AutoRemoveJail & rhs) noexcept + { + using std::swap; + swap(lhs.jid, rhs.jid); + } + + operator int() const + { + return jid; + } + ~AutoRemoveJail(); - void cancel(); - void reset(int j); + + /** + * Remove the jail and cancel this `AutoRemoveJail`, so jail removal is not + * attempted a second time by the destructor. + * + * The destructor calls this ignoring any exception. + */ + void remove(); + + /** + * Cancel the jail removal. + */ + void cancel() noexcept; }; } // namespace nix diff --git a/src/libutil/include/nix/util/file-system.hh b/src/libutil/include/nix/util/file-system.hh index f2b3f7574987..ce042d5059ae 100644 --- a/src/libutil/include/nix/util/file-system.hh +++ b/src/libutil/include/nix/util/file-system.hh @@ -332,15 +332,37 @@ public: x.del = false; } + AutoDelete & operator=(AutoDelete && x) noexcept + { + swap(*this, x); + return *this; + } + + friend void swap(AutoDelete & lhs, AutoDelete & rhs) noexcept + { + using std::swap; + swap(lhs._path, rhs._path); + swap(lhs.del, rhs.del); + swap(lhs.recursive, rhs.recursive); + } + AutoDelete(const std::filesystem::path & p, bool recursive = true); AutoDelete(const AutoDelete &) = delete; - AutoDelete & operator=(AutoDelete &&) = delete; AutoDelete & operator=(const AutoDelete &) = delete; ~AutoDelete(); - void cancel(); + /** + * Delete the file the path points to, and cancel this `AutoDelete`, + * so deletion is not attempted a second time by the destructor. + * + * The destructor calls this, but ignoring any exception. + */ + void deletePath(); - void reset(const std::filesystem::path & p, bool recursive = true); + /** + * Cancel the pending deletion + */ + void cancel() noexcept; const std::filesystem::path & path() const { @@ -520,11 +542,11 @@ private: #ifdef __FreeBSD__ class AutoUnmount { - Path path; + std::filesystem::path path; bool del; public: AutoUnmount(); - AutoUnmount(Path &); + AutoUnmount(const std::filesystem::path &); AutoUnmount(const AutoUnmount &) = delete; AutoUnmount(AutoUnmount && other) noexcept @@ -535,13 +557,31 @@ public: AutoUnmount & operator=(AutoUnmount && other) noexcept { - path = std::move(other.path); - del = std::exchange(other.del, false); + swap(*this, other); return *this; } + friend void swap(AutoUnmount & lhs, AutoUnmount & rhs) noexcept + { + using std::swap; + swap(lhs.path, rhs.path); + swap(lhs.del, rhs.del); + } + ~AutoUnmount(); - void cancel(); + + /** + * Cancel the unmounting + */ + void cancel() noexcept; + + /** + * Unmount the mountpoint right away (if it exists), resetting the + * `AutoUnmount` + * + * The destructor calls this, but ignoring any exception. + */ + void unmount(); }; #endif