From 2e54725017b3e03d29a3f02ca44fc3f6fd5f9353 Mon Sep 17 00:00:00 2001 From: Gleb Natapov Date: Wed, 29 Apr 2015 18:34:06 +0300 Subject: [PATCH 1/6] rpc: allow handler to return a type without default constructor Currently it is not possible because a type is instantiated before been deserialized. Fix that by allocating space for an object by using std::aligned_storage. --- rpc/rpc_impl.hh | 39 ++++++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/rpc/rpc_impl.hh b/rpc/rpc_impl.hh index cfdca41c79..a2bfb152d6 100644 --- a/rpc/rpc_impl.hh +++ b/rpc/rpc_impl.hh @@ -77,11 +77,31 @@ struct reply_payload_base { template struct reply_payload : reply_payload_base { - T v; + void value_set() { + v_set = true; + } + void value_set(T&& v) { + new (&u.v) T(std::move(v)); + value_set(); + } + union U { + U() {} + ~U() {} + typename std::aligned_storage::type pad; + T v; + } u; + ~reply_payload() { + if (v_set) { + u.v.~T(); + } + } +private: + bool v_set = false; // set it to true when U::v is valid object }; template<> struct reply_payload : reply_payload_base { + void value_set() {} }; template @@ -90,6 +110,7 @@ struct rcv_reply_base : reply_payload { promise p; template void set_value(V&&... v) { + this->value_set(); done = true; p.set_value(std::forward(v)...); } @@ -103,8 +124,8 @@ struct rcv_reply_base : reply_payload { template struct rcv_reply : rcv_reply_base { inline future<> get_reply(typename protocol::client& dst) { - return unmarshall(dst.serializer(), dst.in(), std::tie(this->v)).then([this] { - this->set_value(this->v); + return unmarshall(dst.serializer(), dst.in(), std::tie(this->u.v)).then([this] { + this->set_value(this->u.v); }); } }; @@ -112,8 +133,8 @@ struct rcv_reply : rcv_reply_base { template struct rcv_reply> : rcv_reply_base, T...> { inline future<> get_reply(typename protocol::client& dst) { - return unmarshall(dst.serializer(), dst.in(), ref_tuple(this->v)).then([this] { - this->set_value(this->v); + return unmarshall(dst.serializer(), dst.in(), ref_tuple(this->u.v)).then([this] { + this->set_value(this->u.v); }); } }; @@ -232,10 +253,10 @@ template struct snd_reply : snd_reply_base { snd_reply(id_type xid) : snd_reply_base(xid) {} inline void set_val(std::tuple&& val) { - this->v = std::move(std::get<0>(val)); + this->value_set(std::move(std::get<0>(val))); } inline future<> reply(typename protocol::server::connection& client) { - return marshall(client.serializer(), client.out(), std::tie(this->id, this->v)); + return marshall(client.serializer(), client.out(), std::tie(this->id, this->u.v)); } }; @@ -243,10 +264,10 @@ template struct snd_reply> : snd_reply_base> { snd_reply(id_type xid) : snd_reply_base>(xid) {} inline void set_val(std::tuple&& val) { - this->v = std::move(val); + this->value_set(std::move(val)); } inline future<> reply(typename protocol::server::connection& client) { - return marshall(client.serializer(), client.out(), std::tuple_cat(std::tie(this->id), ref_tuple(this->v))); + return marshall(client.serializer(), client.out(), std::tuple_cat(std::tie(this->id), ref_tuple(this->u.v))); } }; From 992a6ea21eb0b34b9cf12078f67d5ef0018b1e82 Mon Sep 17 00:00:00 2001 From: Calle Wilund Date: Thu, 9 Apr 2015 15:46:36 +0200 Subject: [PATCH 2/6] Collectd: Use initializer lists + declare < and == operators for clang Makes scollectd compile on clang++ 3.5.0 --- core/scollectd.cc | 12 ++++++------ core/scollectd.hh | 6 ++++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/core/scollectd.cc b/core/scollectd.cc index 59b7c9f825..534fe86d89 100644 --- a/core/scollectd.cc +++ b/core/scollectd.cc @@ -35,22 +35,22 @@ #include "core/future-util.hh" #include "net/api.hh" -namespace std { -inline bool operator<(const scollectd::type_instance_id & id1, - const scollectd::type_instance_id & id2) { +bool scollectd::type_instance_id::operator<( + const scollectd::type_instance_id& id2) const { + auto& id1 = *this; return std::tie(id1.plugin(), id1.plugin_instance(), id1.type(), id1.type_instance()) < std::tie(id2.plugin(), id2.plugin_instance(), id2.type(), id2.type_instance()); } -inline bool operator==(const scollectd::type_instance_id & id1, - const scollectd::type_instance_id & id2) { +bool scollectd::type_instance_id::operator==( + const scollectd::type_instance_id & id2) const { + auto& id1 = *this; return std::tie(id1.plugin(), id1.plugin_instance(), id1.type(), id1.type_instance()) == std::tie(id2.plugin(), id2.plugin_instance(), id2.type(), id2.type_instance()); } -} namespace scollectd { diff --git a/core/scollectd.hh b/core/scollectd.hh index f5f4521b43..cac83e66f5 100644 --- a/core/scollectd.hh +++ b/core/scollectd.hh @@ -133,6 +133,8 @@ public: const std::string & type_instance() const { return _type_instance; } + bool operator<(const type_instance_id&) const; + bool operator==(const type_instance_id&) const; private: plugin_id _plugin; plugin_instance_id _plugin_instance; @@ -349,13 +351,13 @@ public: } void types(data_type * p) const override { unpack(_values, [p](Args... args) { - const std::array tmp = { (args)... }; + std::initializer_list tmp = { args... }; std::copy(tmp.begin(), tmp.end(), p); }); } void values(net::packed * p) const override { unpack(_values, [p](Args... args) { - std::array tmp = { (args)... }; + std::initializer_list tmp = { args... }; std::copy(tmp.begin(), tmp.end(), p); }); } From 0a47b5f7c904d9b38f52b66057820f5758ab75c1 Mon Sep 17 00:00:00 2001 From: Gleb Natapov Date: Thu, 30 Apr 2015 12:25:15 +0300 Subject: [PATCH 3/6] rpc: return exception as a future instead of throwing in client send path This simplifies error handling in a client code since it needs to only check future for an exception and do not put 'try' block around a call itself. --- rpc/rpc_impl.hh | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/rpc/rpc_impl.hh b/rpc/rpc_impl.hh index a2bfb152d6..bb408ec4b8 100644 --- a/rpc/rpc_impl.hh +++ b/rpc/rpc_impl.hh @@ -206,6 +206,36 @@ inline auto wait_for_reply(typename protocol::client& dst, return std::move(sent); } +template struct make_send_exception_helper { + auto operator()(Ex&& ex) { + return make_exception_future(std::move(ex)); + } +}; + +template struct make_send_exception_helper> { + auto operator()(Ex&& ex) { + return make_exception_future(std::move(ex)); + } +}; + +template struct make_send_exception_helper { + auto operator()(Ex&& ex) { + return make_exception_future<>(std::move(ex)); + } +}; + +template struct make_send_exception_helper { + auto operator()(Ex&& ex) { + return make_exception_future<>(std::move(ex)); + } +}; + +template +inline auto make_send_exception(Ex&& ex) { + make_send_exception_helper ex_maker; + return ex_maker(std::move(ex)); +} + // Returns lambda that can be used to send rpc messages. // The lambda gets client connection and rpc parameters as arguments, marshalls them sends // to a server and waits for a reply. After receiving reply it unmarshalls it and signal completion @@ -219,7 +249,7 @@ auto send_helper(MsgType t, std::index_sequence) { int _[] = { 0, (assert_type::type>(), 0)... }; (void)_; if (dst.error()) { - throw closed_error(); + return make_send_exception(closed_error()); } // send message From dfa0f1c0034e4d3492a0cb237eed84eb39a857a4 Mon Sep 17 00:00:00 2001 From: Gleb Natapov Date: Thu, 30 Apr 2015 12:25:16 +0300 Subject: [PATCH 4/6] rcp: put client into an error state when connection is broken --- rpc/rpc_impl.hh | 1 + 1 file changed, 1 insertion(+) diff --git a/rpc/rpc_impl.hh b/rpc/rpc_impl.hh index bb408ec4b8..e277a83f08 100644 --- a/rpc/rpc_impl.hh +++ b/rpc/rpc_impl.hh @@ -539,6 +539,7 @@ protocol::client::client(protocol& pro } }); }).finally([this] () { + this->_error = true; this->_write_buf.close(); _outstanding.clear(); }); From a28f0efd9ab882db0dab01024fa8ee44ebfe9755 Mon Sep 17 00:00:00 2001 From: Avi Kivity Date: Thu, 30 Apr 2015 13:51:43 +0300 Subject: [PATCH 5/6] sstring: add iterator range constructor --- core/sstring.hh | 5 +++++ tests/sstring_test.cc | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/core/sstring.hh b/core/sstring.hh index c50006ab7d..d4e26144a2 100644 --- a/core/sstring.hh +++ b/core/sstring.hh @@ -153,6 +153,11 @@ public: basic_sstring(const char_type* b, const char_type* e) : basic_sstring(b, e - b) {} basic_sstring(const std::basic_string& s) : basic_sstring(s.data(), s.size()) {} + template + basic_sstring(InputIterator first, InputIterator last) + : basic_sstring(initialized_later(), std::distance(first, last)) { + std::copy(first, last, begin()); + } ~basic_sstring() noexcept { if (is_external()) { std::free(u.external.str); diff --git a/tests/sstring_test.cc b/tests/sstring_test.cc index 7a1fbc1135..21df8f4821 100644 --- a/tests/sstring_test.cc +++ b/tests/sstring_test.cc @@ -24,6 +24,7 @@ #include #include "core/sstring.hh" +#include BOOST_AUTO_TEST_CASE(test_equality) { BOOST_REQUIRE_EQUAL(sstring("aaa"), sstring("aaa")); @@ -121,3 +122,9 @@ BOOST_AUTO_TEST_CASE(test_erase) { BOOST_REQUIRE_EQUAL(str, "adef"); BOOST_REQUIRE_THROW(str.erase(str.begin() + 5, str.begin() + 6), std::out_of_range); } + +BOOST_AUTO_TEST_CASE(test_ctor_iterator) { + std::list data{{'a', 'b', 'c'}}; + sstring s(data.begin(), data.end()); + BOOST_REQUIRE_EQUAL(s, "abc"); +} From 2ec753596999c1687c507051ed2027b0a2ce32cb Mon Sep 17 00:00:00 2001 From: Gleb Natapov Date: Thu, 30 Apr 2015 18:00:42 +0300 Subject: [PATCH 6/6] rpc: allow handler to receive non default constructable types Currently it is not possible because a type is instantiated before been deserialized. Fix that by allocating space for an object by using std::aligned_storage. --- rpc/rpc_impl.hh | 84 ++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 65 insertions(+), 19 deletions(-) diff --git a/rpc/rpc_impl.hh b/rpc/rpc_impl.hh index e277a83f08..471006376c 100644 --- a/rpc/rpc_impl.hh +++ b/rpc/rpc_impl.hh @@ -43,23 +43,33 @@ inline std::enable_if_t> marshall(Serializer& serial }); } -template -inline std::enable_if_t> unmarshall(Serializer&, input_stream&, std::tuple&&) { +// ArgsReady is a functor that will be called after each element is deserialized. +// It gets element's position in a tuple as a parameter. It is used by argument +// desererialization to mark already deserialized element as containing valid values +// that needs to be destroyed by a destructor. +template +inline std::enable_if_t> unmarshall(Serializer&, input_stream&, std::tuple&&, ArgReady&& argready) { return make_ready_future<>(); } -template -inline std::enable_if_t> unmarshall(Serializer& deserialize, input_stream& in, std::tuple&& args) { +template +inline std::enable_if_t> unmarshall(Serializer& deserialize, input_stream& in, std::tuple&& args, ArgReady&& argready) { // And you may ask yourself "What is that beautiful house?"^H^H^H^H "Why // make_ready_future() here?". And there answer would be to convert // exception thrown by deserialize info a future - return make_ready_future().then([&deserialize, &in, args = std::move(args)] { - return deserialize(in, std::get(args)).then([&deserialize, &in, args = std::move(args)] () mutable { - return unmarshall(deserialize, in, std::move(args)); + return make_ready_future().then([&deserialize, &in, args = std::move(args), argready = std::forward(argready)] () mutable { + return deserialize(in, std::get(args)).then([&deserialize, &in, args = std::move(args), argready = std::forward(argready)] () mutable { + argready(N); + return unmarshall(deserialize, in, std::move(args), std::forward(argready)); }); }); } +template +inline future<> unmarshall(Serializer& deserializer, input_stream& in, std::tuple&& args) { + return unmarshall(deserializer, in, std::move(args), [](std::size_t n){}); +} + // ref_tuple gets tuple and returns another tuple with references to members of received tuple template inline std::tuple ref_tuple_impl(std::tuple& t, std::index_sequence) { @@ -147,13 +157,44 @@ struct rcv_reply : rcv_reply_base { } }; +// structure to hold outgoing message parameters on a client side +// while they are serialized template -struct message { +struct out_message { MsgType t; id_type id = 0; std::tuple args; - message() = default; - message(MsgType xt, id_type xid, T&&... xargs) : t(xt), id(xid), args(std::forward(xargs)...) {} + out_message() = delete; + out_message(MsgType xt, id_type xid, T&&... xargs) : t(xt), id(xid), args(std::forward(xargs)...) {} +}; + +// structure to desrialize incoming message parameters to on a server side +template +struct in_message { + using args_type = std::tuple; + id_type id = 0; + bool ready[sizeof...(T)] = {}; + union U { + U() {} + ~U() {} + typename std::aligned_storage::type storage; + args_type args; + } u; + + void set_ready(std::size_t n) { + assert(n < sizeof...(T)); + ready[n] = true; + } + + template + inline void deleter(std::index_sequence) { + // this contraption calls tuple's element destructor if correspondent ready == true + int _[] = {0, (ready[I] && (std::get(u.args).std::tuple_element::type::~type(), true))...}; (void)_; + } + + ~in_message() { + deleter(std::make_index_sequence()); + } }; template @@ -254,7 +295,7 @@ auto send_helper(MsgType t, std::index_sequence) { // send message auto msg_id = dst.next_message_id(); - auto m = std::make_unique::type...>>(t, msg_id, std::forward(args)...); + auto m = std::make_unique::type...>>(t, msg_id, std::forward(args)...); auto xargs = std::tie(m->t, m->id, std::get(m->args)...); // holds references to all message elements promise<> sent; // will be fulfilled when data is sent auto fsent = sent.get_future(); @@ -338,17 +379,17 @@ inline future<> reply(std::unique_ptr>& r, t // build callback arguments tuple depending on whether it gets client_info as a first parameter template -inline auto make_apply_args(client_info& info, std::unique_ptr>& m, std::enable_if_t = nullptr) { - return std::move(m->args); +inline auto make_apply_args(client_info& info, std::unique_ptr>& m, std::enable_if_t = nullptr) { + return std::move(m->u.args); } template -inline auto make_apply_args(client_info& info, std::unique_ptr>& m, std::enable_if_t = nullptr) { - return std::tuple_cat(std::make_tuple(std::cref(info)), std::move(m->args)); +inline auto make_apply_args(client_info& info, std::unique_ptr>& m, std::enable_if_t = nullptr) { + return std::tuple_cat(std::make_tuple(std::cref(info)), std::move(m->u.args)); } template -inline future>> apply(Func& func, client_info& info, std::unique_ptr>&& m) { +inline future>> apply(Func& func, client_info& info, std::unique_ptr>&& m) { using futurator = futurize; auto r = std::make_unique>(m->id); try { @@ -386,9 +427,14 @@ template, Func&& func) { return [func = lref_to_cref(std::forward(func))](lw_shared_ptr::server::connection> client) mutable { // create message to hold all received values - auto m = std::make_unique::type...>>(); - auto xargs = std::tie(m->id, std::get(m->args)...); // holds reference to all message elements - return unmarshall(client->serializer(), client->in(), std::move(xargs)).then([client, m = std::move(m), &func] () mutable { + auto m = std::make_unique::type...>>(); + auto argready = [mptr = m.get()] (std::size_t n) { + if (n) { // skip first element since it is not part of a message tuple + mptr->set_ready(n - 1); + } + }; + auto xargs = std::tie(m->id, std::get(m->u.args)...); // holds reference to all message elements + return unmarshall(client->serializer(), client->in(), std::move(xargs), std::move(argready)).then([client, m = std::move(m), &func] () mutable { // note: apply is executed asynchronously with regards to networking so we cannot chain futures here by doing "return apply()" apply(func, client->info(), std::move(m)).then([client] (std::unique_ptr>&& r) { client->out_ready() = client->out_ready().then([client, r = std::move(r)] () mutable {