Repository: bitcoin/bitcoin
File: src/httpserver.cpp (C++)

Code before:
             RequestMethodString(hreq->GetRequestMethod()), SanitizeString(hreq->GetURI(), SAFE_CHARS_URI).substr(0, 100), hreq->GetPeer().ToStringAddrPort());

    // Find registered handler for prefix
    std::string strURI = hreq->GetURI();
    std::string path;
    LOCK(g_httppathhandlers_mutex);
    std::vector<HTTPPathHandler>::const_iterator i = pathHandlers.begin();
    std::vector<HTTPPathHandler>::const_iterator iend = pathHandlers.end();
    for (; i != iend; ++i) {
        bool match = false;
        if (i->exactMatch)
            match = (strURI == i->prefix);
        else
            match = strURI.starts_with(i->prefix);
        if (match) {
            path = strURI.substr(i->prefix.size());
            break;
        }
    }

    // Dispatch to worker thread
    if (i != iend) {
        std::unique_ptr<HTTPWorkItem> item(new HTTPWorkItem(std::move(hreq), path, i->handler));
        assert(g_work_queue);
        if (g_work_queue->Enqueue(item.get())) {
            item.release(); /* if true, queue took ownership */
        } else {
            LogWarning("Request rejected because http work queue depth exceeded, it can be increased with the -rpcworkqueue= setting");
            item->req->WriteReply(HTTP_SERVICE_UNAVAILABLE, "Work queue depth exceeded");
        }
    } else {
        hreq->WriteReply(HTTP_NOT_FOUND);
    }
}

/** Callback to reject HTTP requests after shutdown. */
static void http_reject_request_cb(struct evhttp_request* req, void*)
{
    LogDebug(BCLog::HTTP, "Rejecting request while shutting down\n");
    evhttp_send_error(req, HTTP_SERVUNAVAIL, nullptr);
}

/** Event dispatcher thread */
static void ThreadHTTP(struct event_base* base)
{
    util::ThreadRename("http");
    LogDebug(BCLog::HTTP, "Entering http event loop\n");
    event_base_dispatch(base);
    // Event loop will be interrupted by InterruptHTTPServer()
    LogDebug(BCLog::HTTP, "Exited http event loop\n");
}

Review comment (bug):
It's a larger diff, so I'm happy to keep this as-is, but I think this is actually quite a nice use-case for `util::Expected` to improve the ownership semantics of `Enqeue`. Ownership can be transferred unambiguously with:

<details>
<summary>git diff on faa23738fc</summary>

```diff
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index abfcb455e1..de6c837ed2 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -14,11 +14,13 @@
 #include <rpc/protocol.h>
 #include <sync.h>
 #include <util/check.h>
+#include <util/expected.h>
 #include <util/signalinterrupt.h>
 #include <util/strencodings.h>
 #include <util/threadnames.h>
 #include <util/translation.h>
 
+#include <concepts>
 #include <condition_variable>
 #include <cstdio>
 #include <cstdlib>
@@ -89,16 +91,18 @@ public:
     /** Precondition: worker threads have all stopped (they have been joined).
      */
     ~WorkQueue() = default;
-    /** Enqueue a work item */
-    bool Enqueue(WorkItem* item) EXCLUSIVE_LOCKS_REQUIRED(!cs)
+    /** Enqueue a work item. Returns the item back on failure. */
+    template <typename DerivedItem>
+        requires std::derived_from<DerivedItem, WorkItem>
+    util::Expected<void, std::unique_ptr<DerivedItem>> Enqueue(std::unique_ptr<DerivedItem> item) EXCLUSIVE_LOCKS_REQUIRED(!cs)
     {
         LOCK(cs);
         if (!running || queue.size() >= maxDepth) {
-            return false;
+            return util::Unexpected{std::move(item)};
         }
-        queue.emplace_back(std::unique_ptr<WorkItem>(item));
+        queue.push_back(std::move(item));
         cond.notify_one();
-        return true;
+        return {};
     }
     /** Thread function */
     void Run() EXCLUSIVE_LOCKS_REQUIRED(!cs)
@@ -329,11 +333,9 @@ static void http_request_cb(struct evhttp_request* req, void* arg)
     if (i != iend) {
         std::unique_ptr<HTTPWorkItem> item(new HTTPWorkItem(std::move(hreq), path, i->handler));
         assert(g_work_queue);
-        if (g_work_queue->Enqueue(item.get())) {
-            [[maybe_unused]] auto _{item.release()}; /* if true, queue took ownership */
-        } else {
+        if (auto res{g_work_queue->Enqueue(std::move(item))}; !res) {
             LogWarning("Request rejected because http work queue depth exceeded, it can be increased with the -rpcworkqueue= setting");
-            item->req->WriteReply(HTTP_SERVICE_UNAVAILABLE, "Work queue depth exceeded");
+            res.error()->req->WriteReply(HTTP_SERVICE_UNAVAILABLE, "Work queue depth exceeded");
         }
     } else {
         hreq->WriteReply(HTTP_NOT_FOUND);

```
</details>


Code after:
             RequestMethodString(hreq->GetRequestMethod()), SanitizeString(hreq->GetURI(), SAFE_CHARS_URI).substr(0, 100), hreq->GetPeer().ToStringAddrPort());

    // Find registered handler for prefix
    std::string strURI = hreq->GetURI();
    std::string path;
    LOCK(g_httppathhandlers_mutex);
    std::vector<HTTPPathHandler>::const_iterator i = pathHandlers.begin();
    std::vector<HTTPPathHandler>::const_iterator iend = pathHandlers.end();
    for (; i != iend; ++i) {
        bool match = false;
        if (i->exactMatch)
            match = (strURI == i->prefix);
        else
            match = strURI.starts_with(i->prefix);
        if (match) {
            path = strURI.substr(i->prefix.size());
            break;
        }
    }

    // Dispatch to worker thread
    if (i != iend) {
        std::unique_ptr<HTTPWorkItem> item(new HTTPWorkItem(std::move(hreq), path, i->handler));
        assert(g_work_queue);
        if (g_work_queue->Enqueue(item.get())) {
            [[maybe_unused]] auto _{item.release()}; /* if true, queue took ownership */
        } else {
            LogWarning("Request rejected because http work queue depth exceeded, it can be increased with the -rpcworkqueue= setting");
            item->req->WriteReply(HTTP_SERVICE_UNAVAILABLE, "Work queue depth exceeded");
        }
    } else {
        hreq->WriteReply(HTTP_NOT_FOUND);
    }
}

/** Callback to reject HTTP requests after shutdown. */
static void http_reject_request_cb(struct evhttp_request* req, void*)
{
    LogDebug(BCLog::HTTP, "Rejecting request while shutting down\n");
    evhttp_send_error(req, HTTP_SERVUNAVAIL, nullptr);
}

/** Event dispatcher thread */
static void ThreadHTTP(struct event_base* base)
{
    util::ThreadRename("http");
    LogDebug(BCLog::HTTP, "Entering http event loop\n");
    event_base_dispatch(base);
    // Event loop will be interrupted by InterruptHTTPServer()
    LogDebug(BCLog::HTTP, "Exited http event loop\n");
}