Skip to content

Commit 9dbcb2e

Browse files
vasildryanofsky
andcommitted
refactor: Remove DestructorCatcher and AsyncCallable
Use kj::Function instead of std::function to avoid the need for AsyncCallable and DestructorCatcher classes, which were used to work around the requirement that std::function objects need to be copyable. kj::Function does not have this requirement. Change is from bitcoin-core#144 (comment) Co-authored-by: Ryan Ofsky <[email protected]>
1 parent 7159dde commit 9dbcb2e

File tree

4 files changed

+58
-15
lines changed

4 files changed

+58
-15
lines changed

include/mp/proxy-io.h

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@
1414

1515
#include <assert.h>
1616
#include <functional>
17-
#include <optional>
17+
#include <kj/function.h>
1818
#include <map>
1919
#include <memory>
20+
#include <optional>
2021
#include <sstream>
2122
#include <string>
2223

@@ -166,15 +167,15 @@ class EventLoop
166167

167168
//! Run function on event loop thread. Does not return until function completes.
168169
//! Must be called while the loop() function is active.
169-
void post(const std::function<void()>& fn);
170+
void post(kj::Function<void()> fn);
170171

171172
//! Wrapper around EventLoop::post that takes advantage of the
172173
//! fact that callable will not go out of scope to avoid requirement that it
173174
//! be copyable.
174175
template <typename Callable>
175176
void sync(Callable&& callable)
176177
{
177-
post(std::ref(callable));
178+
post(std::forward<Callable>(callable));
178179
}
179180

180181
//! Start asynchronous worker thread if necessary. This is only done if
@@ -214,7 +215,7 @@ class EventLoop
214215
std::thread m_async_thread;
215216

216217
//! Callback function to run on event loop thread during post() or sync() call.
217-
const std::function<void()>* m_post_fn = nullptr;
218+
kj::Function<void()>* m_post_fn = nullptr;
218219

219220
//! Callback functions to run on async thread.
220221
CleanupList m_async_fns;
@@ -282,11 +283,9 @@ struct Waiter
282283
// in the case where a capnp response is sent and a brand new
283284
// request is immediately received.
284285
while (m_fn) {
285-
auto fn = std::move(m_fn);
286-
m_fn = nullptr;
287-
lock.unlock();
288-
fn();
289-
lock.lock();
286+
auto fn = std::move(*m_fn);
287+
m_fn.reset();
288+
Unlock(lock, fn);
290289
}
291290
const bool done = pred();
292291
return done;
@@ -295,7 +294,7 @@ struct Waiter
295294

296295
std::mutex m_mutex;
297296
std::condition_variable m_cv;
298-
std::function<void()> m_fn;
297+
std::optional<kj::Function<void()>> m_fn;
299298
};
300299

301300
//! Object holding network & rpc state associated with either an incoming server

include/mp/type-context.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,7 @@ auto PassField(Priority<1>, TypeList<>, ServerContext& server_context, const Fn&
6464
auto future = kj::newPromiseAndFulfiller<typename ServerContext::CallContext>();
6565
auto& server = server_context.proxy_server;
6666
int req = server_context.req;
67-
auto invoke = MakeAsyncCallable(
68-
[fulfiller = kj::mv(future.fulfiller),
67+
auto invoke = [fulfiller = kj::mv(future.fulfiller),
6968
call_context = kj::mv(server_context.call_context), &server, req, fn, args...]() mutable {
7069
const auto& params = call_context.getParams();
7170
Context::Reader context_arg = Accessor::get(params);
@@ -143,14 +142,14 @@ auto PassField(Priority<1>, TypeList<>, ServerContext& server_context, const Fn&
143142
fulfiller_dispose->reject(kj::mv(*exception));
144143
});
145144
}
146-
});
145+
};
147146

148147
// Lookup Thread object specified by the client. The specified thread should
149148
// be a local Thread::Server object, but it needs to be looked up
150149
// asynchronously with getLocalServer().
151150
auto thread_client = context_arg.getThread();
152151
return server.m_context.connection->m_threads.getLocalServer(thread_client)
153-
.then([&server, invoke, req](const kj::Maybe<Thread::Server&>& perhaps) {
152+
.then([&server, invoke = kj::mv(invoke), req](const kj::Maybe<Thread::Server&>& perhaps) mutable {
154153
// Assuming the thread object is found, pass it a pointer to the
155154
// `invoke` lambda above which will invoke the function on that
156155
// thread.

include/mp/util.h

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ void Unlock(Lock& lock, Callback&& callback)
161161
callback();
162162
}
163163

164+
<<<<<<< HEAD
164165
//! Needed for libc++/macOS compatibility. Lets code work with shared_ptr nothrow declaration
165166
//! https://github.com/capnproto/capnproto/issues/553#issuecomment-328554603
166167
template <typename T>
@@ -201,6 +202,49 @@ AsyncCallable<std::remove_reference_t<Callable>> MakeAsyncCallable(Callable&& ca
201202
return std::forward<Callable>(callable);
202203
}
203204

205+
||||||| parent of 4b02963 (refactor: Remove DestructorCatcher and AsyncCallable)
206+
//! Needed for libc++/macOS compatibility. Lets code work with shared_ptr nothrow declaration
207+
//! https://github.com/capnproto/capnproto/issues/553#issuecomment-328554603
208+
template <typename T>
209+
struct DestructorCatcher
210+
{
211+
T value;
212+
template <typename... Params>
213+
DestructorCatcher(Params&&... params) : value(kj::fwd<Params>(params)...)
214+
{
215+
}
216+
~DestructorCatcher() noexcept try {
217+
} catch (const kj::Exception& e) { // NOLINT(bugprone-empty-catch)
218+
}
219+
};
220+
221+
//! Wrapper around callback function for compatibility with std::async.
222+
//!
223+
//! std::async requires callbacks to be copyable and requires noexcept
224+
//! destructors, but this doesn't work well with kj types which are generally
225+
//! move-only and not noexcept.
226+
template <typename Callable>
227+
struct AsyncCallable
228+
{
229+
AsyncCallable(Callable&& callable) : m_callable(std::make_shared<DestructorCatcher<Callable>>(std::move(callable)))
230+
{
231+
}
232+
AsyncCallable(const AsyncCallable&) = default;
233+
AsyncCallable(AsyncCallable&&) = default;
234+
~AsyncCallable() noexcept = default;
235+
ResultOf<Callable> operator()() const { return (m_callable->value)(); }
236+
mutable std::shared_ptr<DestructorCatcher<Callable>> m_callable;
237+
};
238+
239+
//! Construct AsyncCallable object.
240+
template <typename Callable>
241+
AsyncCallable<std::remove_reference_t<Callable>> MakeAsyncCallable(Callable&& callable)
242+
{
243+
return std::move(callable);
244+
}
245+
246+
=======
247+
>>>>>>> 4b02963 (refactor: Remove DestructorCatcher and AsyncCallable)
204248
//! Format current thread name as "{exe_name}-{$pid}/{thread_name}-{$tid}".
205249
std::string ThreadName(const char* exe_name);
206250

src/mp/proxy.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <kj/common.h>
2323
#include <kj/debug.h>
2424
#include <kj/exception.h>
25+
#include <kj/function.h>
2526
#include <kj/memory.h>
2627
#include <map>
2728
#include <memory>
@@ -246,7 +247,7 @@ void EventLoop::loop()
246247
m_post_fd = -1;
247248
}
248249

249-
void EventLoop::post(const std::function<void()>& fn)
250+
void EventLoop::post(kj::Function<void()> fn)
250251
{
251252
if (std::this_thread::get_id() == m_thread_id) {
252253
fn();

0 commit comments

Comments
 (0)