refactor: avoid evbuffer use in tr_web, tr_webseed (#7743)

* refactor: add Task::add_data()

* refactor: move response body preallocation to Task ctor

* refactor: make Task::body() private

* refactor: add FetchOptions::on_data_received callback

* refactor: remove FetchOptions::buffer

* refactor: remove evbuffer from web.cc

* refactor: remove evbuffer from webseed.cc

* refactor: remove unused evbuffer_unique_ptr

* fix: copypaste error

Xref: https://github.com/transmission/transmission/pull/7743\#discussion_r2516592222

libtransmission/utils-ev.cc

@@ -3,7 +3,6 @@
 // or any future license endorsed by Mnemosyne LLC.
 // License text can be found in the licenses/ folder.
 
-#include <event2/buffer.h>
 #include <event2/event.h>
 #include <event2/http.h>
 
@@ -12,14 +11,6 @@
 namespace libtransmission::evhelpers
 {
 
-void BufferDeleter::operator()(struct evbuffer* buf) const noexcept
-{
-    if (buf != nullptr)
-    {
-        evbuffer_free(buf);
-    }
-}
-
 void EventBaseDeleter::operator()(struct event_base* evbase) const noexcept
 {
     if (evbase != nullptr)

libtransmission/utils-ev.h

@@ -11,7 +11,6 @@
 
 #include <memory>
 
-struct evbuffer;
 struct event;
 struct event_base;
 struct evhttp;
@@ -19,13 +18,6 @@ struct evhttp;
 namespace libtransmission::evhelpers
 {
 
-struct BufferDeleter
-{
-    void operator()(struct evbuffer* buf) const noexcept;
-};
-
-using evbuffer_unique_ptr = std::unique_ptr<struct evbuffer, BufferDeleter>;
-
 struct EventBaseDeleter
 {
     void operator()(struct event_base* evbase) const noexcept;

libtransmission/web.cc

@@ -33,8 +33,6 @@
 
 #include <curl/curl.h>
 
-#include <event2/buffer.h>
-
 #include <fmt/format.h>
 
 #ifdef _WIN32
@@ -42,7 +40,6 @@
 #endif
 #include "libtransmission/log.h"
 #include "libtransmission/tr-assert.h"
-#include "libtransmission/utils-ev.h"
 #include "libtransmission/utils.h"
 #include "libtransmission/web.h"
 #include "libtransmission/web-utils.h"
@@ -264,6 +261,13 @@ public:
             easy_ = parsed ? impl.get_easy(parsed->host) : nullptr;
 
             response.user_data = options_.done_func_user_data;
+
+            if (options_.range)
+            {
+                // preallocate the response body buffer
+                auto const& [first, last] = *options_.range;
+                response.body.reserve(last + 1U - first);
+            }
         }
 
         // Some of the curl_easy_setopt() args took a pointer to this task.
@@ -283,11 +287,6 @@ public:
             return easy_;
         }
 
-        [[nodiscard]] auto* body() const
-        {
-            return options_.buffer != nullptr ? options_.buffer : privbuf_.get();
-        }
-
         [[nodiscard]] constexpr auto const& speedLimitTag() const
         {
             return options_.speed_limit_tag;
@@ -355,6 +354,17 @@ public:
             }
         }
 
+        void add_data(void const* data, size_t const n_bytes)
+        {
+            response.body.append(static_cast<char const*>(data), n_bytes);
+            tr_logAddTrace(fmt::format("wrote {} bytes to task {}'s buffer", n_bytes, fmt::ptr(this)));
+
+            if (options_.on_data_received)
+            {
+                options_.on_data_received(n_bytes);
+            }
+        }
+
         void done()
         {
             if (!options_.done_func)
@@ -362,7 +372,6 @@ public:
                 return;
             }
 
-            response.body.assign(reinterpret_cast<char const*>(evbuffer_pullup(body(), -1)), evbuffer_get_length(body()));
             impl.mediator.run(std::move(options_.done_func), std::move(this->response));
             options_.done_func = {};
         }
@@ -396,8 +405,6 @@ public:
             }
         }
 
-        libtransmission::evhelpers::evbuffer_unique_ptr privbuf_{ evbuffer_new() };
-
         tr_web::FetchOptions options_;
 
         CURL* easy_;
@@ -522,8 +529,7 @@ public:
             }
         }
 
-        evbuffer_add(task->body(), data, bytes_used);
+        task->add_data(data, bytes_used);
-        tr_logAddTrace(fmt::format("wrote {} bytes to task {}'s buffer", bytes_used, fmt::ptr(task)));
         return bytes_used;
     }
 
@@ -641,12 +647,9 @@ public:
             (void)curl_easy_setopt(e, CURLOPT_HTTP_CONTENT_DECODING, 0L);
 
             // set the range request
-            auto const [first, last] = *range;
+            auto const& [first, last] = *range;
-            auto const range_str = fmt::format("{:d}-{:d}", first, last);
+            auto const str = fmt::format("{:d}-{:d}", first, last);
-            (void)curl_easy_setopt(e, CURLOPT_RANGE, range_str.c_str());
+            (void)curl_easy_setopt(e, CURLOPT_RANGE, str.c_str());
-
-            // preallocate the response body buffer
-            evbuffer_expand(task.body(), last + 1U - first);
         }
 
         if (curl_avoid_http2)

libtransmission/web.h

@@ -16,8 +16,6 @@
 #include <string_view>
 #include <utility>
 
-struct evbuffer;
-
 class tr_web
 {
 public:
@@ -85,10 +83,9 @@ public:
         // Maximum time to wait before timeout
         std::chrono::seconds timeout_secs = DefaultTimeoutSecs;
 
-        // If provided, this buffer will be used to hold the response body.
+        // Called periodically by the web internals when data is received.
-        // Provided for webseeds, which need to set low-level callbacks on
+        // Used by webseeds to report to tr_bandwidth for data xfer stats
-        // the buffer itself.
+        std::function<void(size_t /*n_bytes*/)> on_data_received;
-        evbuffer* buffer = nullptr;
 
         // IP protocol to use when making the request
         IPProtocol ip_proto = IPProtocol::ANY;

libtransmission/webseed.cc

@@ -14,8 +14,6 @@
 #include <string_view>
 #include <utility>
 
-#include <event2/buffer.h>
-
 #include <fmt/format.h>
 
 #include "libtransmission/transmission.h"
@@ -30,16 +28,14 @@
 #include "libtransmission/timer.h"
 #include "libtransmission/torrent.h"
 #include "libtransmission/tr-assert.h"
+#include "libtransmission/tr-buffer.h"
 #include "libtransmission/tr-macros.h"
 #include "libtransmission/tr-strbuf.h"
-#include "libtransmission/utils-ev.h"
 #include "libtransmission/utils.h"
 #include "libtransmission/web-utils.h"
 #include "libtransmission/web.h"
 #include "libtransmission/webseed.h"
 
-struct evbuffer;
-
 using namespace std::literals;
 using namespace libtransmission::Values;
 
@@ -57,12 +53,6 @@ public:
         , end_byte_{ tor.block_loc(blocks.end - 1).byte + tor.block_size(blocks.end - 1) }
         , loc_{ tor.block_loc(blocks.begin) }
     {
-        evbuffer_add_cb(content_.get(), on_buffer_got_data, this);
-    }
-
-    [[nodiscard]] auto* content() const
-    {
-        return content_.get();
     }
 
     void request_next_chunk();
@@ -74,7 +64,7 @@ private:
     void use_fetched_blocks();
 
     static void on_partial_data_fetched(tr_web::FetchResponse const& web_response);
-    static void on_buffer_got_data(evbuffer* /*buf*/, evbuffer_cb_info const* info, void* vtask);
+    void on_data_received(size_t n_bytes);
 
     tr_webseed_impl* const webseed_;
     tr_session* const session_;
@@ -83,7 +73,7 @@ private:
     // the current position in the task; i.e., the next block to save
     tr_block_info::Location loc_;
 
-    libtransmission::evhelpers::evbuffer_unique_ptr const content_{ evbuffer_new() };
+    libtransmission::StackBuffer<tr_block_info::BlockSize, std::byte, std::ratio<5, 1>> content_;
 };
 
 /**
@@ -377,22 +367,22 @@ void tr_webseed_task::use_fetched_blocks()
 
     auto const& tor = webseed_->tor;
 
-    for (auto* const buf = content();;)
+    for (;;)
     {
         auto const block_size = tor.block_size(loc_.block);
-        if (evbuffer_get_length(buf) < block_size)
+        if (std::size(content_) < block_size)
         {
             break;
         }
 
         if (tor.has_block(loc_.block))
         {
-            evbuffer_drain(buf, block_size);
+            content_.drain(block_size);
         }
         else
         {
             auto block_buf = new Cache::BlockData(block_size);
-            evbuffer_remove(buf, std::data(*block_buf), std::size(*block_buf));
+            content_.to_buf(std::data(*block_buf), std::size(*block_buf));
             session_->run_in_session_thread(
                 [session = session_, tor_id = tor.id(), block = loc_.block, block_buf, webseed = webseed_]()
                 {
@@ -415,17 +405,15 @@ void tr_webseed_task::use_fetched_blocks()
 
 // ---
 
-void tr_webseed_task::on_buffer_got_data(evbuffer* /*buf*/, evbuffer_cb_info const* info, void* vtask)
+void tr_webseed_task::on_data_received(size_t const n_bytes)
 {
-    size_t const n_added = info->n_added;
+    if (n_bytes == 0 || dead)
-    auto* const task = static_cast<tr_webseed_task*>(vtask);
-    if (n_added == 0 || task->dead)
     {
         return;
     }
 
-    auto const lock = task->session_->unique_lock();
+    auto const lock = session_->unique_lock();
-    task->webseed_->got_piece_data(n_added);
+    webseed_->got_piece_data(n_bytes);
 }
 
 void tr_webseed_task::on_partial_data_fetched(tr_web::FetchResponse const& web_response)
@@ -452,6 +440,7 @@ void tr_webseed_task::on_partial_data_fetched(tr_web::FetchResponse const& web_r
         return;
     }
 
+    task->content_.add(std::data(body), std::size(body));
     task->use_fetched_blocks();
 
     if (task->loc_.byte < task->end_byte_)
@@ -463,7 +452,7 @@ void tr_webseed_task::on_partial_data_fetched(tr_web::FetchResponse const& web_r
         return;
     }
 
-    TR_ASSERT(evbuffer_get_length(task->content()) == 0);
+    TR_ASSERT(std::empty(task->content_));
     TR_ASSERT(task->loc_.byte == task->end_byte_);
     webseed->tasks.erase(task);
     delete task;
@@ -488,7 +477,7 @@ void tr_webseed_task::request_next_chunk()
 {
     auto const& tor = webseed_->tor;
 
-    auto const downloaded_loc = tor.byte_loc(loc_.byte + evbuffer_get_length(content()));
+    auto const downloaded_loc = tor.byte_loc(loc_.byte + std::size(content_));
 
     auto const [file_index, file_offset] = tor.file_offset(downloaded_loc);
     auto const left_in_file = tor.file_size(file_index) - file_offset;
@@ -503,7 +492,10 @@ void tr_webseed_task::request_next_chunk()
     auto options = tr_web::FetchOptions{ url.sv(), on_partial_data_fetched, this };
     options.range.emplace(file_offset, file_offset + this_chunk - 1);
     options.speed_limit_tag = tor.id();
-    options.buffer = content();
+    options.on_data_received = [this](size_t const n_bytes)
+    {
+        on_data_received(n_bytes);
+    };
     tor.session->fetch(std::move(options));
 }