lix-project/lix
18
47
Fork 24
Code Issues 333 Code Review (Gerrit) Projects 1 Releases Wiki Activity

Fix a hang in nix-copy-ssh.sh

Browse source 
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.
This commit is contained in:
Eelco Dolstra 2022-08-23 14:14:47 +02:00
parent f865048332
commit f0358ed465
1 changed files with 14 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
src/libstore/store-api.cc
View file

@ -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;