aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEelco Dolstra <edolstra@gmail.com>2022-08-23 14:14:47 +0200
committerEelco Dolstra <edolstra@gmail.com>2022-08-23 14:19:53 +0200
commitf0358ed4650e4608a383bd9f59ee23545f86c4ad (patch)
tree049cf9a43d964630e918231816e9903b708bb1f6
parentf865048332a26f69a007881877203b9428783357 (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.
-rw-r--r--src/libstore/store-api.cc17
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;