diff options
author | Eelco Dolstra <edolstra@gmail.com> | 2022-08-23 14:14:47 +0200 |
---|---|---|
committer | Eelco Dolstra <edolstra@gmail.com> | 2022-08-23 14:19:53 +0200 |
commit | f0358ed4650e4608a383bd9f59ee23545f86c4ad (patch) | |
tree | 049cf9a43d964630e918231816e9903b708bb1f6 /src | |
parent | f865048332a26f69a007881877203b9428783357 (diff) |
Fix a hang in nix-copy-ssh.sh
This hang for some reason didn't trigger in the Nix build, but did
running 'make installcheck' interactively. What happened:
* Store::addMultipleToStore() calls a SinkToSource object to copy a
path, which in turn calls LegacySSHStore::narFromPath(), which
acquires a connection.
* The SinkToSource object is not destroyed after the last bytes has
been read, so the coroutine's stack is still alive and its
destructors are not run. So the connection is not released.
* Then when the next path is copied, because max-connections = 1,
LegacySSHStore::narFromPath() hangs forever waiting for a connection
to be released.
The fix is to make sure that the source object is destroyed when we're
done with it.
Diffstat (limited to 'src')
-rw-r--r-- | src/libstore/store-api.cc | 17 |
1 files changed, 14 insertions, 3 deletions
diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 1406bf657..9c3a0b3d6 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -11,6 +11,7 @@ #include "archive.hh" #include "callback.hh" #include "remote-store.hh" +#include "finally.hh" #include <regex> @@ -271,7 +272,7 @@ void Store::addMultipleToStore( using PathWithInfo = std::pair<ValidPathInfo, std::unique_ptr<Source>>; - std::map<StorePath, PathWithInfo*> infosMap; + std::map<StorePath, PathWithInfo *> infosMap; StorePathSet storePathsToAdd; for (auto & thingToAdd : pathsToCopy) { infosMap.insert_or_assign(thingToAdd.first.path, &thingToAdd); @@ -288,7 +289,8 @@ void Store::addMultipleToStore( storePathsToAdd, [&](const StorePath & path) { - auto & [info, source] = *infosMap.at(path); + + auto & [info, _] = *infosMap.at(path); if (isValidPath(info.path)) { nrDone++; @@ -309,12 +311,21 @@ void Store::addMultipleToStore( auto info = info_; info.ultimate = false; + /* Make sure that the Source object is destroyed when + we're done. In particular, a SinkToSource object must + be destroyed to ensure that the destructors on its + stack frame are run; this includes + LegacySSHStore::narFromPath()'s connection lock. */ + Finally cleanupSource{[&]() { + source.reset(); + }}; + if (!isValidPath(info.path)) { MaintainCount<decltype(nrRunning)> mc(nrRunning); showProgress(); try { addToStore(info, *source, repair, checkSigs); - } catch (Error &e) { + } catch (Error & e) { nrFailed++; if (!settings.keepGoing) throw e; |