From 1c28270c9d18e3a4913f70d62428b43973aa256a Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Mon, 4 Nov 2024 18:47:02 +0100 Subject: [PATCH] libstore: don't hold state lock while unpausing transfers libcurl may call callbacks during unpause. if libcurl calls these callbacks often enough they may find that they've exhausted their allotted buffer space, at which point they will call dataCallback as provided in enqueueFileTransfer. download() provides callbacks that have their own state locks and may call unpause on transfers with *their* state locks they share with curlFileTransfer. it was possible to cause a deadlock between these two if the curl worker thread tried to provide some data to a callback with the transfer state lock held and the user transfer simultaneously unpaused its associated curl transfer, with its own state lock held. obviously not a great situation, but avoidable by not holding any lock when we unpause transfers and execute their callbacks, as is otherwise the case when a transfer runs normally and is never paused at all Change-Id: I58556d292adaf7dfb14001d3a6c5c38fa71994da --- src/libstore/filetransfer.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index dcb3b7817..ef914ddeb 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -632,11 +632,14 @@ struct curlFileTransfer : public FileTransfer timeoutMs = INT64_MAX; { - auto state(state_.lock()); - for (auto & item : state->unpause) { + auto unpause = [&] { return std::move(state_.lock()->unpause); }(); + for (auto & item : unpause) { curl_easy_pause(item->req, CURLPAUSE_CONT); } - state->unpause.clear(); + } + + { + auto state(state_.lock()); while (!state->incoming.empty()) { auto item = state->incoming.top(); if (item->embargo <= now) {