From f0358ed4650e4608a383bd9f59ee23545f86c4ad Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Tue, 23 Aug 2022 14:14:47 +0200
Subject: 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.
---
 src/libstore/store-api.cc | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

(limited to 'src')

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;
-- 
cgit v1.2.3