http: Track active requests and wait for last to finish - 2nd attempt #26742

fjahr · 2022-12-22T18:48:58Z

This revives #19420. Since promag is not so active at the moment, I can support this to finally get it merged.

The PR is rebased and comments by jonatack have been addressed.

Once this is merged, I will also reopen #19434.

DrahtBot · 2022-12-22T18:49:01Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	stickies-v, hebasto
Concept ACK	brunoerg, promag
Stale ACK	kevkevinpal

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

kevkevinpal · 2022-12-23T04:50:20Z

Running test/functional/feature_abortnode.py before and after change we get a significant performance gain

running before changes f3bc1a7

test/functional/feature_abortnode.py
2022-12-23T04:47:19.826000Z TestFramework (INFO): Initializing test directory /var/folders/9g/cvx014yx4dq5lwl_x5zvn_j80000gn/T/bitcoin_func_test_k3_6xric
2022-12-23T04:47:21.229000Z TestFramework (INFO): Waiting for crash
2022-12-23T04:47:51.429000Z TestFramework (INFO): Node crashed - now verifying restart fails
2022-12-23T04:47:52.086000Z TestFramework (INFO): Stopping nodes
2022-12-23T04:47:52.300000Z TestFramework (INFO): Cleaning up /var/folders/9g/cvx014yx4dq5lwl_x5zvn_j80000gn/T/bitcoin_func_test_k3_6xric on exit
2022-12-23T04:47:52.300000Z TestFramework (INFO): Tests successful

32.50 seconds to run test

running test with these changes applied 40528e7

test/functional/feature_abortnode.py
2022-12-23T04:44:30.075000Z TestFramework (INFO): Initializing test directory /var/folders/9g/cvx014yx4dq5lwl_x5zvn_j80000gn/T/bitcoin_func_test_0h3w_rbx
2022-12-23T04:44:31.516000Z TestFramework (INFO): Waiting for crash
2022-12-23T04:44:31.837000Z TestFramework (INFO): Node crashed - now verifying restart fails
2022-12-23T04:44:32.452000Z TestFramework (INFO): Stopping nodes
2022-12-23T04:44:32.727000Z TestFramework (INFO): Cleaning up /var/folders/9g/cvx014yx4dq5lwl_x5zvn_j80000gn/T/bitcoin_func_test_0h3w_rbx on exit
2022-12-23T04:44:32.727000Z TestFramework (INFO): Tests successful

1.2 seconds to run test

stickies-v

Concept ACK - thanks for reviving this!

brunoerg

Concept ACK

src/httpserver.cpp

promag · 2022-12-23T15:12:10Z

Concept ACK

src/httpserver.cpp

kevkevinpal · 2022-12-27T01:57:09Z

retACK f95063e with test/functional/feature_abortnode.py

hebasto · 2022-12-27T16:24:13Z

From #15363 (comment):

The libevent HTTP interface is kind of crazy, so I don't think it's surprising that fixing this requires some trial and error. But it would be good to have tests for each issue that gets fixed to avoid going in circles.

Suggesting to add a test which will prevent any regressions in the future.

src/httpserver.cpp

promag · 2023-01-03T17:48:01Z

From #15363 (comment):

The libevent HTTP interface is kind of crazy, so I don't think it's surprising that fixing this requires some trial and error. But it would be good to have tests for each issue that gets fixed to avoid going in circles.

Suggesting to add a test which will prevent any regressions in the future.

@hebasto like

bitcoin/test/functional/feature_abortnode.py

Line 44 in 7bb07bf

self.nodes[0].wait_until_stopped(timeout=200)

should take less than 5s or so?

fjahr · 2023-01-05T12:47:09Z

From #15363 (comment):

The libevent HTTP interface is kind of crazy, so I don't think it's surprising that fixing this requires some trial and error. But it would be good to have tests for each issue that gets fixed to avoid going in circles.

Suggesting to add a test which will prevent any regressions in the future.

@hebasto like

bitcoin/test/functional/feature_abortnode.py

Line 44 in 7bb07bf

self.nodes[0].wait_until_stopped(timeout=200)

should take less than 5s or so?

I have updated the test and it seems to be reliably failing on master, so I think this is a good enough regression test @hebasto ?

Also addressed other comments by @hebasto .

hebasto

Approach ACK 6d3a848.

src/httpserver.cpp

hebasto · 2023-01-05T14:23:57Z

src/httpserver.cpp

+        WAIT_LOCK(g_requests_mutex, lock);
+        g_requests_cv.wait(lock, []() EXCLUSIVE_LOCKS_REQUIRED(g_requests_mutex) {
+                if (!g_requests.empty()) {
+                    LogPrint(BCLog::HTTP, "Waiting for %d requests to stop HTTP server\n", g_requests.size());


Having side effects in the predicate seems a bad idea due to spurious wakeups. It should be a pure function.

Sure, then I think this isn't possible anymore so I am reverting back to how I did it before. I think that's fine though.

stickies-v

Approach ACK 6d3a848

Can anyone please confirm or correct my understanding of why this so much faster?

My current understanding is that even when all of the workers in g_thread_http_workers have terminated (i.e. the entire queue is processed and running==false), the http server thread g_thread_http does not return (when joining) immediately because there are still some idle connections (with whom?) open, preventing the server from shutting down. By first (instead of afterwards) calling evhttp_free(eventHTTP), we can safely destroy the server even if connections are still open (but all requests are handled), causing g_thread_http.join() to return immediately in StopHTTPServer() as opposed to waiting for a timeout.

I think the commit and PR message would benefit from a more clear reason why this works.

stickies-v · 2023-01-05T16:36:56Z

src/httpserver.cpp

+    // Track requests and notify when a request is completed.
+    {
+        WITH_LOCK(g_requests_mutex, g_requests.insert(req));
+        g_requests_cv.notify_all();


What's the purpose of this notification? I think we (currently) only need to monitor for completed requests, not new ones?

Did you see #26742 (comment) ?

I hadn't seen that, thanks for linking.

It's a minor concern, but I'm a bit worried that we're creating a very general contract ("g_requests_cv will always notify for any changes to the g_requests") on a global variable when it's enforced outside of g_requests, when in the future we might easily introduce mutations on g_requests without notifying the cv. A more scoped cv, such as g_requests_cv_handled largely mitigates that.

But, again, no big concern, and it's not likely to cause any major problems should we miss something there. As you see fit.

FYI I think one approach to improve on this contract (and as a nice benefit, also encapsulate code a bit better - I think this improves readability) in the below diff.

(WaitUntilEmpty() needs improvement because it shouldn't directly be logging about the HTTP server like that. I guess we also don't need a GlobalMutex here.)

git diff on 55bbf04

diff --git a/src/httpserver.cpp b/src/httpserver.cpp index b481ef093..90ef37bd5 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -134,6 +134,40 @@ struct HTTPPathHandler HTTPRequestHandler handler; }; +class RequestTracker +{ +public: + typedef std::unordered_set<evhttp_request*> requests_t; +private: + GlobalMutex m_requests_mutex; + std::condition_variable m_requests_cv; + requests_t m_requests GUARDED_BY(m_requests_mutex); +public: + void Register(struct evhttp_request* req) + { + WITH_LOCK(m_requests_mutex, m_requests.insert(req)); + m_requests_cv.notify_all(); + } + + size_t Remove(struct evhttp_request* req) + { + auto n{WITH_LOCK(m_requests_mutex, return m_requests.erase(req))}; + m_requests_cv.notify_all(); + return n; + } + + void WaitUntilEmpty() + { + WAIT_LOCK(m_requests_mutex, lock); + if (!m_requests.empty()) { + LogPrint(BCLog::HTTP, "Waiting for %d requests to stop HTTP server\n", m_requests.size()); + } + m_requests_cv.wait(lock, [this]() EXCLUSIVE_LOCKS_REQUIRED(m_requests_mutex) { + return m_requests.empty(); + }); + } +}; + /** HTTP module state */ //! libevent event loop @@ -149,10 +183,8 @@ static GlobalMutex g_httppathhandlers_mutex; static std::vector<HTTPPathHandler> pathHandlers GUARDED_BY(g_httppathhandlers_mutex); //! Bound listening sockets static std::vector<evhttp_bound_socket *> boundSockets; -//! Track active requests -static GlobalMutex g_requests_mutex; -static std::condition_variable g_requests_cv; -static std::unordered_set<evhttp_request*> g_requests GUARDED_BY(g_requests_mutex); +static RequestTracker g_request_tracker; + /** Check if a network address is allowed to access the HTTP server */ static bool ClientAllowed(const CNetAddr& netaddr) @@ -216,12 +248,9 @@ static void http_request_cb(struct evhttp_request* req, void* arg) { // Track requests and notify when a request is completed. { - WITH_LOCK(g_requests_mutex, g_requests.insert(req)); - g_requests_cv.notify_all(); + g_request_tracker.Register(req); evhttp_request_set_on_complete_cb(req, [](struct evhttp_request* req, void*) { - auto n = WITH_LOCK(g_requests_mutex, return g_requests.erase(req)); - assert(n == 1); - g_requests_cv.notify_all(); + assert(g_request_tracker.Remove(req) == 1); }, nullptr); } @@ -477,15 +506,7 @@ void StopHTTPServer() evhttp_del_accept_socket(eventHTTP, socket); } boundSockets.clear(); - { - WAIT_LOCK(g_requests_mutex, lock); - if (!g_requests.empty()) { - LogPrint(BCLog::HTTP, "Waiting for %d requests to stop HTTP server\n", g_requests.size()); - } - g_requests_cv.wait(lock, []() EXCLUSIVE_LOCKS_REQUIRED(g_requests_mutex) { - return g_requests.empty(); - }); - } + g_request_tracker.WaitUntilEmpty(); if (eventHTTP) { // Schedule a callback to call evhttp_free in the event base thread, so // that evhttp_free does not need to be called again after the handling

Sorry but it's a significant change and this PR already has a pretty long history and several ACKs for the current approach. I would suggest you open a follow-up PR with this as an additional improvement.

src/httpserver.cpp

fjahr · 2023-01-09T12:52:18Z

Can anyone please confirm or correct my understanding of why this so much faster?

My current understanding is that even when all of the workers in g_thread_http_workers have terminated (i.e. the entire queue is processed and running==false), the http server thread g_thread_http does not return (when joining) immediately because there are still some idle connections (with whom?) open, preventing the server from shutting down. By first (instead of afterwards) calling evhttp_free(eventHTTP), we can safely destroy the server even if connections are still open (but all requests are handled), causing g_thread_http.join() to return immediately in StopHTTPServer() as opposed to waiting for a timeout.

Sounds good to me. I guess libevent doesn't close the idle connection as an optimization (less overhead if the client wants to send more requests) and if the client doesn't close them then we are seeing this issue. I would add that the overhead that we see in the functional test is the timeout we have configured on the http server ourselves here.

promag · 2023-01-09T14:27:30Z

I guess libevent doesn't close the idle connection as an optimization (less overhead if the client wants to send more requests) and if the client doesn't close them then we are seeing this issue.

Not really an optimization of libevent, AuthServiceProxy used in the test framework creates a persistent connection (to reuse the same connection for consecutive HTTP requests, as @fjahr says).

promag

ACK. With this change evhttp_free is called after all requests are completed. Calling evhttp_free at this point will cause all active connections to close and nothing remains on the event base, and all RPC threads are ready to be joined. As mentioned above, this is useful to avoid hitting the idle timeout.

Note that shutdown still hangs while a request is in-flight.

src/httpserver.cpp

stickies-v

ACK 55bbf04

I have some open suggestions, but I'm happy with the PR as it is.

src/httpserver.cpp

stickies-v · 2023-01-11T11:43:08Z

src/httpserver.cpp

+    // Track requests and notify when a request is completed.
+    {
+        WITH_LOCK(g_requests_mutex, g_requests.insert(req));
+        g_requests_cv.notify_all();


FYI I think one approach to improve on this contract (and as a nice benefit, also encapsulate code a bit better - I think this improves readability) in the below diff.

(WaitUntilEmpty() needs improvement because it shouldn't directly be logging about the HTTP server like that. I guess we also don't need a GlobalMutex here.)

git diff on 55bbf04

diff --git a/src/httpserver.cpp b/src/httpserver.cpp index b481ef093..90ef37bd5 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -134,6 +134,40 @@ struct HTTPPathHandler HTTPRequestHandler handler; }; +class RequestTracker +{ +public: + typedef std::unordered_set<evhttp_request*> requests_t; +private: + GlobalMutex m_requests_mutex; + std::condition_variable m_requests_cv; + requests_t m_requests GUARDED_BY(m_requests_mutex); +public: + void Register(struct evhttp_request* req) + { + WITH_LOCK(m_requests_mutex, m_requests.insert(req)); + m_requests_cv.notify_all(); + } + + size_t Remove(struct evhttp_request* req) + { + auto n{WITH_LOCK(m_requests_mutex, return m_requests.erase(req))}; + m_requests_cv.notify_all(); + return n; + } + + void WaitUntilEmpty() + { + WAIT_LOCK(m_requests_mutex, lock); + if (!m_requests.empty()) { + LogPrint(BCLog::HTTP, "Waiting for %d requests to stop HTTP server\n", m_requests.size()); + } + m_requests_cv.wait(lock, [this]() EXCLUSIVE_LOCKS_REQUIRED(m_requests_mutex) { + return m_requests.empty(); + }); + } +}; + /** HTTP module state */ //! libevent event loop @@ -149,10 +183,8 @@ static GlobalMutex g_httppathhandlers_mutex; static std::vector<HTTPPathHandler> pathHandlers GUARDED_BY(g_httppathhandlers_mutex); //! Bound listening sockets static std::vector<evhttp_bound_socket *> boundSockets; -//! Track active requests -static GlobalMutex g_requests_mutex; -static std::condition_variable g_requests_cv; -static std::unordered_set<evhttp_request*> g_requests GUARDED_BY(g_requests_mutex); +static RequestTracker g_request_tracker; + /** Check if a network address is allowed to access the HTTP server */ static bool ClientAllowed(const CNetAddr& netaddr) @@ -216,12 +248,9 @@ static void http_request_cb(struct evhttp_request* req, void* arg) { // Track requests and notify when a request is completed. { - WITH_LOCK(g_requests_mutex, g_requests.insert(req)); - g_requests_cv.notify_all(); + g_request_tracker.Register(req); evhttp_request_set_on_complete_cb(req, [](struct evhttp_request* req, void*) { - auto n = WITH_LOCK(g_requests_mutex, return g_requests.erase(req)); - assert(n == 1); - g_requests_cv.notify_all(); + assert(g_request_tracker.Remove(req) == 1); }, nullptr); } @@ -477,15 +506,7 @@ void StopHTTPServer() evhttp_del_accept_socket(eventHTTP, socket); } boundSockets.clear(); - { - WAIT_LOCK(g_requests_mutex, lock); - if (!g_requests.empty()) { - LogPrint(BCLog::HTTP, "Waiting for %d requests to stop HTTP server\n", g_requests.size()); - } - g_requests_cv.wait(lock, []() EXCLUSIVE_LOCKS_REQUIRED(g_requests_mutex) { - return g_requests.empty(); - }); - } + g_request_tracker.WaitUntilEmpty(); if (eventHTTP) { // Schedule a callback to call evhttp_free in the event base thread, so // that evhttp_free does not need to be called again after the handling

src/httpserver.cpp

This was made obsolete by tracking the active requests and explicitly waiting for them to finish before shutdown.

fjahr · 2023-02-10T19:47:13Z

Addressed comments by @promag and @stickies-v

stickies-v

re-ACK 60978c8

hebasto

ACK 60978c8

hebasto · 2023-03-01T13:39:04Z

src/httpserver.cpp


 #include <sys/types.h>
 #include <sys/stat.h>

 #include <event2/buffer.h>
 #include <event2/bufferevent.h>
 #include <event2/http.h>
+#include <event2/http_struct.h>


nit: IWYU suggests to s/#include <event2/http_struct.h>/#include <event2/event.h>/.

Thanks! I will fix this in the follow-up that will pick up #19434.

achow101 · 2023-03-07T00:29:49Z

ACK 60978c8

… finish - 2nd attempt 60978c8 test: Reduce extended timeout on abortnode test (Fabian Jahr) 660bdbf http: Release server before waiting for event base loop exit (João Barbosa) 8c6d007 http: Track active requests and wait for last to finish (João Barbosa) Pull request description: This revives bitcoin#19420. Since promag is not so active at the moment, I can support this to finally get it merged. The PR is rebased and comments by jonatack have been addressed. Once this is merged, I will also reopen bitcoin#19434. ACKs for top commit: achow101: ACK 60978c8 stickies-v: re-ACK [60978c8](bitcoin@60978c8) hebasto: ACK 60978c8 Tree-SHA512: eef0fe1081e9331b95cfafc71d82f2398abd1d3439dac5b2fa5c6d9c0a3f63ef19adde1c38c88d3b4e7fb41ce7c097943f1815c10e33d165918ccbdec512fe1c

…lient disconnection 68f23f5 http: bugfix: track closed connection (stickies-v) 084d037 http: log connection instead of request count (stickies-v) 41f9027 http: refactor: use encapsulated HTTPRequestTracker (stickies-v) Pull request description: #26742 significantly increased the http server shutdown speed, but also introduced a bug (#27722 - see #27722 (comment) for steps to reproduce on master) that causes http server shutdown to halt in case of a remote client disconnection. This happens because `evhttp_request_set_on_complete_cb` is never called and thus the request is never removed from `g_requests`. This PR fixes that bug, and improves robustness of the code by encapsulating the request tracking logic. Earlier approaches (#27909, #27245, #19434) attempted to resolve this but [imo are fundamentally unsafe](#27909 (comment)) because of differences in lifetime between an `evhttp_request` and `evhttp_connection`. We don't need to keep track of open requests or connections, we just [need to ensure](#19420 (comment)) that there are no active requests on server shutdown. Because a connection can have multiple requests, and a request can be completed in various ways (the request actually being handled, or the client performing a remote disconnect), keeping a counter per connection seems like the approach with the least overhead to me. Fixes #27722 ACKs for top commit: vasild: ACK 68f23f5 theStack: ACK 68f23f5 Tree-SHA512: dfa711ff55ec75ba44d73e9e6fac16b0be25cf3c20868c2145a844a7878ad9fc6998d9ff62d72f3a210bfa411ef03d3757b73d68a7c22926e874c421e51444d6

…emote client disconnection 68f23f5 http: bugfix: track closed connection (stickies-v) 084d037 http: log connection instead of request count (stickies-v) 41f9027 http: refactor: use encapsulated HTTPRequestTracker (stickies-v) Pull request description: bitcoin#26742 significantly increased the http server shutdown speed, but also introduced a bug (bitcoin#27722 - see bitcoin#27722 (comment) for steps to reproduce on master) that causes http server shutdown to halt in case of a remote client disconnection. This happens because `evhttp_request_set_on_complete_cb` is never called and thus the request is never removed from `g_requests`. This PR fixes that bug, and improves robustness of the code by encapsulating the request tracking logic. Earlier approaches (bitcoin#27909, bitcoin#27245, bitcoin#19434) attempted to resolve this but [imo are fundamentally unsafe](bitcoin#27909 (comment)) because of differences in lifetime between an `evhttp_request` and `evhttp_connection`. We don't need to keep track of open requests or connections, we just [need to ensure](bitcoin#19420 (comment)) that there are no active requests on server shutdown. Because a connection can have multiple requests, and a request can be completed in various ways (the request actually being handled, or the client performing a remote disconnect), keeping a counter per connection seems like the approach with the least overhead to me. Fixes bitcoin#27722 ACKs for top commit: vasild: ACK 68f23f5 theStack: ACK 68f23f5 Tree-SHA512: dfa711ff55ec75ba44d73e9e6fac16b0be25cf3c20868c2145a844a7878ad9fc6998d9ff62d72f3a210bfa411ef03d3757b73d68a7c22926e874c421e51444d6

DrahtBot added the RPC/REST/ZMQ label Dec 22, 2022

fjahr mentioned this pull request Dec 22, 2022

http: Track active requests and wait for last to finish #19420

Closed

stickies-v reviewed Dec 23, 2022

View reviewed changes

brunoerg reviewed Dec 23, 2022

View reviewed changes

src/httpserver.cpp Outdated Show resolved Hide resolved

fjahr force-pushed the 202212-pr19420 branch from 40528e7 to b9b8b5d Compare December 23, 2022 23:06

promag reviewed Dec 23, 2022

View reviewed changes

src/httpserver.cpp Show resolved Hide resolved

promag reviewed Dec 23, 2022

View reviewed changes

src/httpserver.cpp Outdated Show resolved Hide resolved

src/httpserver.cpp Outdated Show resolved Hide resolved

fjahr force-pushed the 202212-pr19420 branch 2 times, most recently from 77a6ea8 to f95063e Compare December 25, 2022 18:56

promag reviewed Dec 26, 2022

View reviewed changes

src/httpserver.cpp Outdated Show resolved Hide resolved

hebasto reviewed Dec 27, 2022

View reviewed changes

src/httpserver.cpp Outdated Show resolved Hide resolved

src/httpserver.cpp Outdated Show resolved Hide resolved

src/httpserver.cpp Outdated Show resolved Hide resolved

src/httpserver.cpp Outdated Show resolved Hide resolved

hebasto reviewed Dec 27, 2022

View reviewed changes

src/httpserver.cpp Outdated Show resolved Hide resolved

fjahr force-pushed the 202212-pr19420 branch from f95063e to 6d3a848 Compare January 5, 2023 12:44

hebasto reviewed Jan 5, 2023

View reviewed changes

stickies-v reviewed Jan 6, 2023

View reviewed changes

fjahr force-pushed the 202212-pr19420 branch from 6d3a848 to 55bbf04 Compare January 9, 2023 12:25

promag reviewed Jan 9, 2023

View reviewed changes

promag reviewed Jan 11, 2023

View reviewed changes

src/httpserver.cpp Outdated Show resolved Hide resolved

stickies-v approved these changes Jan 11, 2023

View reviewed changes

http: Track active requests and wait for last to finish

8c6d007

promag and others added 2 commits February 10, 2023 20:35

http: Release server before waiting for event base loop exit

660bdbf

test: Reduce extended timeout on abortnode test

60978c8

This was made obsolete by tracking the active requests and explicitly waiting for them to finish before shutdown.

fjahr force-pushed the 202212-pr19420 branch from 55bbf04 to 60978c8 Compare February 10, 2023 19:45

stickies-v approved these changes Feb 11, 2023

View reviewed changes

DrahtBot requested a review from kevkevinpal February 11, 2023 12:36

hebasto approved these changes Mar 1, 2023

View reviewed changes

bitcoin deleted a comment Mar 4, 2023

fanquake requested review from john-moffett and removed request for kevkevinpal March 5, 2023 09:45

DrahtBot requested a review from kevkevinpal March 6, 2023 12:49

achow101 merged commit 86bacd7 into bitcoin:master Mar 7, 2023

Crypt-iQ mentioned this pull request May 22, 2023

bitcoind hangs waiting for g_requests.empty() #27722

Closed

1 task

stickies-v mentioned this pull request Sep 29, 2023

http: bugfix: allow server shutdown in case of remote client disconnection #28551

Merged

bitcoin locked and limited conversation to collaborators Mar 6, 2024

http: Track active requests and wait for last to finish - 2nd attempt #26742

http: Track active requests and wait for last to finish - 2nd attempt #26742

Uh oh!

Conversation

fjahr commented Dec 22, 2022

Uh oh!

DrahtBot commented Dec 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Uh oh!

kevkevinpal commented Dec 23, 2022

Uh oh!

stickies-v left a comment

Choose a reason for hiding this comment

Uh oh!

brunoerg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

promag commented Dec 23, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kevkevinpal commented Dec 27, 2022

Uh oh!

hebasto commented Dec 27, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

promag commented Jan 3, 2023

Uh oh!

fjahr commented Jan 5, 2023

Uh oh!

hebasto left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stickies-v left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fjahr commented Jan 9, 2023

Uh oh!

promag commented Jan 9, 2023

Uh oh!

promag left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stickies-v left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fjahr commented Feb 10, 2023

Uh oh!

stickies-v left a comment

DrahtBot commented Dec 22, 2022 •

edited

Loading

promag left a comment •

edited

Loading