diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index c9f5d6260..aa8f4be1d 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -708,7 +708,7 @@ struct curlFileTransfer : public FileTransfer download thread and the calling thread. */ struct State { - bool quit = false; + bool done = false, failed = false; std::exception_ptr exc; std::string data; std::condition_variable avail, request; @@ -717,18 +717,17 @@ struct curlFileTransfer : public FileTransfer auto _state = std::make_shared>(); - /* In case of an exception, wake up the download thread. FIXME: - abort the download request. */ + /* In case of an exception, wake up the download thread. */ Finally finally([&]() { auto state(_state->lock()); - state->quit = true; + state->failed |= std::uncaught_exceptions() != 0; state->request.notify_one(); }); enqueueFileTransfer(request, {[_state](std::future fut) { auto state(_state->lock()); - state->quit = true; + state->done = true; try { fut.get(); } catch (...) { @@ -740,7 +739,10 @@ struct curlFileTransfer : public FileTransfer [_state, &sink](TransferItem & transfer, std::string_view data) { auto state(_state->lock()); - if (state->quit) return; + if (state->failed) { + // actual exception doesn't matter, the other end is already dead + throw std::exception{}; + } if (!state->decompressor) { state->decompressor = makeDecompressionSink(transfer.encoding, sink); @@ -775,7 +777,7 @@ struct curlFileTransfer : public FileTransfer if (state->data.empty()) { - if (state->quit) { + if (state->done) { if (state->exc) std::rethrow_exception(state->exc); if (state->decompressor) { state->decompressor->finish(); diff --git a/tests/unit/libstore/filetransfer.cc b/tests/unit/libstore/filetransfer.cc new file mode 100644 index 000000000..192bf81ef --- /dev/null +++ b/tests/unit/libstore/filetransfer.cc @@ -0,0 +1,32 @@ +#include "filetransfer.hh" + +#include +#include + +using namespace std::chrono_literals; + +namespace nix { + +TEST(FileTransfer, exceptionAbortsDownload) +{ + struct Done + {}; + + auto ft = makeFileTransfer(); + + LambdaSink broken([](auto block) { throw Done(); }); + + ASSERT_THROW(ft->download(FileTransferRequest("file:///dev/zero"), broken), Done); + + // makeFileTransfer returns a ref<>, which cannot be cleared. since we also + // can't default-construct it we'll have to overwrite it instead, but we'll + // take the raw pointer out first so we can destroy it in a detached thread + // (otherwise a failure will stall the process and have it killed by meson) + auto reset = std::async(std::launch::async, [&]() { ft = makeFileTransfer(); }); + EXPECT_EQ(reset.wait_for(10s), std::future_status::ready); + // if this did time out we have to leak `reset`. + if (reset.wait_for(0s) == std::future_status::timeout) { + (void) new auto(std::move(reset)); + } +} +} diff --git a/tests/unit/meson.build b/tests/unit/meson.build index 339ac9a4a..f5355cce8 100644 --- a/tests/unit/meson.build +++ b/tests/unit/meson.build @@ -113,6 +113,7 @@ libstore_tests_sources = files( 'libstore/derivation.cc', 'libstore/derived-path.cc', 'libstore/downstream-placeholder.cc', + 'libstore/filetransfer.cc', 'libstore/machines.cc', 'libstore/nar-info-disk-cache.cc', 'libstore/outputs-spec.cc',