From 23b7f02100b08f1c35c669f78c507a0b4a071780 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 29 Dec 2021 13:20:09 -0600 Subject: [PATCH] fix: clear verify_thread before releasing verify mutex lock (#2360) * fix: clear verify_thread before releasing verify mutex lock * refactor: increase the verify buffer size * fix: uniqueness comparison for verify torrents --- libtransmission/verify.cc | 59 ++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/libtransmission/verify.cc b/libtransmission/verify.cc index e218818e0..36bc3d9fe 100644 --- a/libtransmission/verify.cc +++ b/libtransmission/verify.cc @@ -7,10 +7,10 @@ */ #include -#include /* free() */ #include #include #include +#include #include "transmission.h" #include "completion.h" @@ -31,6 +31,8 @@ static auto constexpr MsecToSleepPerSecondDuringVerify = int{ 100 }; static bool verifyTorrent(tr_torrent* tor, bool* stopFlag) { + auto const begin = tr_time(); + tr_sys_file_t fd = TR_BAD_SYS_FILE; uint64_t filePos = 0; bool changed = false; @@ -40,11 +42,8 @@ static bool verifyTorrent(tr_torrent* tor, bool* stopFlag) tr_file_index_t fileIndex = 0; tr_file_index_t prevFileIndex = !fileIndex; tr_piece_index_t piece = 0; - time_t const begin = tr_time(); - size_t const buflen = 1024 * 128; // 128 KiB buffer - auto* const buffer = static_cast(tr_malloc(buflen)); - - tr_sha1_ctx_t sha = tr_sha1_init(); + auto buffer = std::vector(1024 * 256); + auto sha = tr_sha1_init(); tr_logAddTorDbg(tor, "%s", "verifying torrent..."); tor->verify_progress = 0; @@ -73,16 +72,16 @@ static bool verifyTorrent(tr_torrent* tor, bool* stopFlag) uint64_t leftInPiece = tor->pieceSize(piece) - piecePos; uint64_t leftInFile = file_length - filePos; uint64_t bytesThisPass = std::min(leftInFile, leftInPiece); - bytesThisPass = std::min(bytesThisPass, uint64_t{ buflen }); + bytesThisPass = std::min(bytesThisPass, uint64_t(std::size(buffer))); /* read a bit */ if (fd != TR_BAD_SYS_FILE) { auto numRead = uint64_t{}; - if (tr_sys_file_read_at(fd, buffer, bytesThisPass, filePos, &numRead, nullptr) && numRead > 0) + if (tr_sys_file_read_at(fd, std::data(buffer), bytesThisPass, filePos, &numRead, nullptr) && numRead > 0) { bytesThisPass = numRead; - tr_sha1_update(sha, buffer, bytesThisPass); + tr_sha1_update(sha, std::data(buffer), bytesThisPass); tr_sys_file_advise(fd, filePos, bytesThisPass, TR_SYS_FILE_ADVICE_DONT_NEED, nullptr); } } @@ -143,7 +142,6 @@ static bool verifyTorrent(tr_torrent* tor, bool* stopFlag) tor->verify_progress.reset(); tr_sha1_final(sha); - free(buffer); /* stopwatch */ time_t const end = tr_time(); @@ -184,6 +182,12 @@ struct verify_node return current_size < that.current_size ? -1 : 1; } + // tertiary compare just to ensure they don't compare equal + if (torrent->infoHash() != that.torrent->infoHash()) + { + return torrent->infoHash() < that.torrent->infoHash() ? -1 : 1; + } + return 0; } @@ -195,8 +199,8 @@ struct verify_node static struct verify_node currentNode; // TODO: refactor s.t. this doesn't leak -static auto& verifyList{ *new std::set{} }; -static tr_thread* verifyThread = nullptr; +static auto& verify_list{ *new std::set{} }; +static tr_thread* verify_thread = nullptr; static bool stopCurrent = false; static std::mutex verify_mutex_; @@ -205,27 +209,26 @@ static void verifyThreadFunc(void* /*user_data*/) { for (;;) { - bool changed = false; - { auto const lock = std::lock_guard(verify_mutex_); stopCurrent = false; - if (std::empty(verifyList)) + if (std::empty(verify_list)) { currentNode.torrent = nullptr; - break; + verify_thread = nullptr; + return; } - auto const it = std::begin(verifyList); + auto const it = std::begin(verify_list); currentNode = *it; - verifyList.erase(it); + verify_list.erase(it); } tr_torrent* tor = currentNode.torrent; tr_logAddTorInfo(tor, "%s", _("Verifying torrent")); tor->setVerifyState(TR_VERIFY_NOW); - changed = verifyTorrent(tor, &stopCurrent); + auto const changed = verifyTorrent(tor, &stopCurrent); tor->setVerifyState(TR_VERIFY_NONE); TR_ASSERT(tr_isTorrent(tor)); @@ -239,8 +242,6 @@ static void verifyThreadFunc(void* /*user_data*/) (*currentNode.callback_func)(tor, stopCurrent, currentNode.callback_data); } } - - verifyThread = nullptr; } void tr_verifyAdd(tr_torrent* tor, tr_verify_done_func callback_func, void* callback_data) @@ -256,11 +257,11 @@ void tr_verifyAdd(tr_torrent* tor, tr_verify_done_func callback_func, void* call auto const lock = std::lock_guard(verify_mutex_); tor->setVerifyState(TR_VERIFY_WAIT); - verifyList.insert(node); + verify_list.insert(node); - if (verifyThread == nullptr) + if (verify_thread == nullptr) { - verifyThread = tr_threadNew(verifyThreadFunc, nullptr); + verify_thread = tr_threadNew(verifyThreadFunc, nullptr); } } @@ -284,20 +285,20 @@ void tr_verifyRemove(tr_torrent* tor) else { auto const it = std::find_if( - std::begin(verifyList), - std::end(verifyList), + std::begin(verify_list), + std::end(verify_list), [tor](auto const& task) { return tor == task.torrent; }); tor->setVerifyState(TR_VERIFY_NONE); - if (it != std::end(verifyList)) + if (it != std::end(verify_list)) { if (it->callback_func != nullptr) { (*it->callback_func)(tor, true, it->callback_data); } - verifyList.erase(it); + verify_list.erase(it); } } @@ -309,5 +310,5 @@ void tr_verifyClose(tr_session* /*session*/) auto const lock = std::lock_guard(verify_mutex_); stopCurrent = true; - verifyList.clear(); + verify_list.clear(); }