isapego commented on code in PR #7607: URL: https://github.com/apache/ignite-3/pull/7607#discussion_r2925165185
########## modules/platforms/cpp/tests/fake_server/proxy/asio_proxy.h: ########## @@ -0,0 +1,266 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#pragma once + +#include <atomic> +#include <iostream> +#include <map> +#include <memory> +#include <queue> +#include <thread> +#include <vector> + +#include <asio.hpp> +#include <asio/ts/internet.hpp> + +#include "message.h" +#include "message_listener.h" + +namespace ignite::proxy { + +using asio::ip::tcp; + +struct configuration { + asio::ip::port_type m_in_port; + std::string m_out_host_and_port; + message_listener* m_listener; + + configuration(asio::ip::port_type m_in_port, const std::string &m_out_host_and_port, message_listener *m_listener) + : m_in_port(m_in_port) + , m_out_host_and_port(m_out_host_and_port) + , m_listener(m_listener) { } +}; + +class session : public std::enable_shared_from_this<session> { +public: + session(tcp::socket in_sock, tcp::socket out_sock, std::atomic_bool& stopped, message_listener* listener) + : m_in_sock(std::move(in_sock)) + , m_out_sock(std::move(out_sock)) + , m_stopped(stopped) + , m_listener(listener) { } + + void start() { do_serve(); } + + tcp::socket &get_out_sock() { return m_out_sock; } + + void set_writable(bool writable) { + m_in_to_out_writable = writable; + m_out_to_in_writable = writable; + } + + enum direction { forward, reverse }; + +private: + void do_serve() { + do_read(forward); + do_read(reverse); + } + + void do_read(direction direction) { + if (m_stopped.load()) + return; + + tcp::socket &src = direction == forward ? m_in_sock : m_out_sock; + + auto self(shared_from_this()); + + src.async_read_some(asio::buffer(buf, BUFF_SIZE), + [direction, self](const asio::error_code& ec, size_t len) { + if (ec) { + if (ec == asio::error::eof) { + return; + } + throw std::runtime_error("Error while reading from socket " + ec.message()); + } + + std::queue<message> &queue = direction == forward ? self->m_in_to_out : self->m_out_to_in; + bool &writable = direction == forward ? self->m_in_to_out_writable : self->m_out_to_in_writable; + + // we have one-threaded executor no synchronization is needed + message& msg = queue.emplace(self->buf, len); + + if (self->m_listener) { + if (direction == forward) { + self->m_listener->register_out_message(msg); + } else { + self->m_listener->register_in_message(msg); + } + } + + if (writable) { // there are pending write operation on this socket + self->do_write(direction); + } + + self->do_read(direction); + }); + } + + void do_write(direction direction) { + tcp::socket &dst = direction == forward ? m_out_sock : m_in_sock; + std::queue<message> &queue = direction == forward ? m_in_to_out : m_out_to_in; + bool &writable = direction == forward ? m_in_to_out_writable : m_out_to_in_writable; + + writable = false; // protects from writing same buffer twice (from head of queue). + + auto self(shared_from_this()); + if (!queue.empty()) { + message &msg = queue.front(); + + asio::async_write( + dst, asio::buffer(msg.m_arr, msg.m_size), + [direction, self](asio::error_code ec, size_t) { + if (ec) { + if (ec == asio::error::eof) { + return; + } + throw std::runtime_error("Error while writing to socket " + ec.message()); + } + + std::queue<message> &queue = direction == forward ? self->m_in_to_out : self->m_out_to_in; + bool &writable = direction == forward ? self->m_in_to_out_writable : self->m_out_to_in_writable; + + queue.pop(); + + if (!queue.empty()) { + // makes writes on the same socket strictly ordered + self->do_write(direction); + } else { + writable = true; // now read operation can initiate writes + } + }); + } + } + + tcp::socket m_in_sock; + tcp::socket m_out_sock; + + bool m_in_to_out_writable{false}; + bool m_out_to_in_writable{false}; + + std::queue<message> m_in_to_out; + std::queue<message> m_out_to_in; + + static constexpr size_t BUFF_SIZE = 4096; + + char buf[BUFF_SIZE]{}; Review Comment: Let's use `std::array` here instead. ########## modules/platforms/cpp/cmake/dependencies.cmake: ########## @@ -43,6 +43,24 @@ function(fetch_dependency NAME URL MD5) endif() endfunction() +function(add_asio_dependency) + message(STATUS "Download dependency: asio") + + FetchContent_Declare( + asio + URL https://github.com/chriskohlhoff/asio/archive/refs/tags/asio-1-36-0.tar.gz + URL_HASH MD5=6699ac1dea111c20d024f25e06e573db Review Comment: Yeah, let's actually do this. ########## modules/platforms/cpp/tests/fake_server/proxy/asio_proxy.h: ########## @@ -0,0 +1,266 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#pragma once + +#include <atomic> +#include <iostream> +#include <map> +#include <memory> +#include <queue> +#include <thread> +#include <vector> + +#include <asio.hpp> +#include <asio/ts/internet.hpp> + +#include "message.h" +#include "message_listener.h" + +namespace ignite::proxy { + +using asio::ip::tcp; + +struct configuration { + asio::ip::port_type m_in_port; + std::string m_out_host_and_port; + message_listener* m_listener; + + configuration(asio::ip::port_type m_in_port, const std::string &m_out_host_and_port, message_listener *m_listener) + : m_in_port(m_in_port) + , m_out_host_and_port(m_out_host_and_port) + , m_listener(m_listener) { } +}; + +class session : public std::enable_shared_from_this<session> { +public: + session(tcp::socket in_sock, tcp::socket out_sock, std::atomic_bool& stopped, message_listener* listener) + : m_in_sock(std::move(in_sock)) + , m_out_sock(std::move(out_sock)) + , m_stopped(stopped) + , m_listener(listener) { } + + void start() { do_serve(); } + + tcp::socket &get_out_sock() { return m_out_sock; } + + void set_writable(bool writable) { + m_in_to_out_writable = writable; + m_out_to_in_writable = writable; + } + + enum direction { forward, reverse }; Review Comment: Let's use `enum class` instead ```suggestion enum class direction { forward, reverse }; ``` ########## modules/platforms/cpp/tests/fake_server/proxy/message.h: ########## @@ -0,0 +1,62 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +// + +#pragma once + +#include <cstring> +#include <utility> + +namespace ignite::proxy { + +struct message { + char *m_arr{nullptr}; + size_t m_size = 0; + + friend void swap(message& lhs, message& rhs) noexcept { + using std::swap; + swap(lhs.m_arr, rhs.m_arr); + swap(lhs.m_size, rhs.m_size); + } + + message(char *arr, size_t size) + : m_size(size) + { + m_arr = new char[m_size]; + std::memcpy(m_arr, arr, size); + } Review Comment: I actually agree with copilot here. Why not using `std::vector`? ########## modules/platforms/cpp/tests/fake_server/proxy/asio_proxy.h: ########## @@ -0,0 +1,266 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#pragma once + +#include <atomic> +#include <iostream> +#include <map> +#include <memory> +#include <queue> +#include <thread> +#include <vector> + +#include <asio.hpp> +#include <asio/ts/internet.hpp> + +#include "message.h" +#include "message_listener.h" + +namespace ignite::proxy { + +using asio::ip::tcp; + +struct configuration { + asio::ip::port_type m_in_port; + std::string m_out_host_and_port; + message_listener* m_listener; + + configuration(asio::ip::port_type m_in_port, const std::string &m_out_host_and_port, message_listener *m_listener) + : m_in_port(m_in_port) + , m_out_host_and_port(m_out_host_and_port) + , m_listener(m_listener) { } +}; + +class session : public std::enable_shared_from_this<session> { +public: + session(tcp::socket in_sock, tcp::socket out_sock, std::atomic_bool& stopped, message_listener* listener) + : m_in_sock(std::move(in_sock)) + , m_out_sock(std::move(out_sock)) + , m_stopped(stopped) + , m_listener(listener) { } + + void start() { do_serve(); } + + tcp::socket &get_out_sock() { return m_out_sock; } + + void set_writable(bool writable) { + m_in_to_out_writable = writable; + m_out_to_in_writable = writable; + } + + enum direction { forward, reverse }; + +private: + void do_serve() { + do_read(forward); + do_read(reverse); + } + + void do_read(direction direction) { + if (m_stopped.load()) + return; + + tcp::socket &src = direction == forward ? m_in_sock : m_out_sock; + + auto self(shared_from_this()); + + src.async_read_some(asio::buffer(buf, BUFF_SIZE), + [direction, self](const asio::error_code& ec, size_t len) { + if (ec) { + if (ec == asio::error::eof) { + return; + } + throw std::runtime_error("Error while reading from socket " + ec.message()); + } + + std::queue<message> &queue = direction == forward ? self->m_in_to_out : self->m_out_to_in; + bool &writable = direction == forward ? self->m_in_to_out_writable : self->m_out_to_in_writable; + + // we have one-threaded executor no synchronization is needed + message& msg = queue.emplace(self->buf, len); Review Comment: Wait, if every message refers to the same buffer, why do we even need a queue? Also, changing a buffer will affect all the messages, which does not look right to me. This logic here needs explanation. ########## modules/platforms/cpp/tests/fake_server/proxy/asio_proxy.h: ########## @@ -0,0 +1,260 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#pragma once + +#include <iostream> +#include <queue> +#include <tuple> +#include <map> + +#include <asio.hpp> +#include <asio/ts/buffer.hpp> +#include <asio/ts/internet.hpp> + +#include "message.h" +#include "message_listener.h" + +namespace ignite::proxy { + +using asio::ip::tcp; + +struct configuration { + asio::ip::port_type m_in_port; + std::string m_out_host_and_port; + message_listener* m_listener; + + configuration(asio::ip::port_type m_in_port, const std::string &m_out_host_and_port, message_listener *m_listener) + : m_in_port(m_in_port) + , m_out_host_and_port(m_out_host_and_port) + , m_listener(m_listener) { } +}; + +class session : public std::enable_shared_from_this<session> { +public: + session(tcp::socket in_sock, tcp::socket out_sock, std::atomic_bool& stopped, message_listener* listener) + : m_in_sock(std::move(in_sock)) + , m_out_sock(std::move(out_sock)) + , m_stopped(stopped) + , m_listener(listener) + { } + + ~session() { + std::cout << "Session destructed " << this << std::endl; + } + + void start() { do_serve(); } + + tcp::socket &get_out_sock() { return m_out_sock; } + + void set_writable(bool writable) { + m_in_to_out_writable = writable; + m_out_to_in_writable = writable; + } + + enum direction { forward, reverse }; + +private: + void do_serve() { + do_read(forward); + do_read(reverse); + } + + void do_read(direction direction) { + if (m_stopped.load()) + return; + + tcp::socket &src = direction == forward ? m_in_sock : m_out_sock; + std::queue<message> &queue = direction == forward ? m_in_to_out : m_out_to_in; + bool &writable = direction == forward ? m_in_to_out_writable : m_out_to_in_writable; + + auto self(shared_from_this()); + + src.async_read_some(asio::buffer(buf, BUFF_SIZE), + [&queue, direction, &writable, self](const asio::error_code& ec, size_t len) { + if (ec) { + if (ec == asio::error::eof) { + return; + } + throw std::runtime_error("Error while reading from socket " + ec.message()); + } Review Comment: Can you explain a bit? Why is it intended? Seems to me like read error is something that can happen and probably should not crash the process? ########## modules/platforms/cpp/tests/fake_server/proxy/asio_proxy.h: ########## @@ -0,0 +1,266 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#pragma once + +#include <atomic> +#include <iostream> +#include <map> +#include <memory> +#include <queue> +#include <thread> +#include <vector> + +#include <asio.hpp> +#include <asio/ts/internet.hpp> + +#include "message.h" +#include "message_listener.h" + +namespace ignite::proxy { + +using asio::ip::tcp; + +struct configuration { + asio::ip::port_type m_in_port; + std::string m_out_host_and_port; + message_listener* m_listener; + + configuration(asio::ip::port_type m_in_port, const std::string &m_out_host_and_port, message_listener *m_listener) + : m_in_port(m_in_port) + , m_out_host_and_port(m_out_host_and_port) + , m_listener(m_listener) { } +}; + +class session : public std::enable_shared_from_this<session> { +public: + session(tcp::socket in_sock, tcp::socket out_sock, std::atomic_bool& stopped, message_listener* listener) + : m_in_sock(std::move(in_sock)) + , m_out_sock(std::move(out_sock)) + , m_stopped(stopped) + , m_listener(listener) { } + + void start() { do_serve(); } + + tcp::socket &get_out_sock() { return m_out_sock; } + + void set_writable(bool writable) { + m_in_to_out_writable = writable; + m_out_to_in_writable = writable; + } + + enum direction { forward, reverse }; + +private: + void do_serve() { + do_read(forward); + do_read(reverse); + } + + void do_read(direction direction) { + if (m_stopped.load()) + return; Review Comment: Why do we even need this flag here? What will happen if we remove it? ########## modules/platforms/cpp/tests/fake_server/proxy/asio_proxy.h: ########## @@ -0,0 +1,266 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#pragma once + +#include <atomic> +#include <iostream> +#include <map> +#include <memory> +#include <queue> +#include <thread> +#include <vector> + +#include <asio.hpp> +#include <asio/ts/internet.hpp> + +#include "message.h" +#include "message_listener.h" + +namespace ignite::proxy { + +using asio::ip::tcp; + +struct configuration { + asio::ip::port_type m_in_port; + std::string m_out_host_and_port; + message_listener* m_listener; Review Comment: Let's use `std::shared_ptr` here to avoid unnecessary debugging in future. ```suggestion std::shared_ptr<message_listener> m_listener; ``` ########## modules/platforms/cpp/tests/fake_server/proxy/asio_proxy.h: ########## @@ -0,0 +1,266 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#pragma once + +#include <atomic> +#include <iostream> +#include <map> +#include <memory> +#include <queue> +#include <thread> +#include <vector> + +#include <asio.hpp> +#include <asio/ts/internet.hpp> + +#include "message.h" +#include "message_listener.h" + +namespace ignite::proxy { + +using asio::ip::tcp; + +struct configuration { + asio::ip::port_type m_in_port; + std::string m_out_host_and_port; + message_listener* m_listener; + + configuration(asio::ip::port_type m_in_port, const std::string &m_out_host_and_port, message_listener *m_listener) + : m_in_port(m_in_port) + , m_out_host_and_port(m_out_host_and_port) + , m_listener(m_listener) { } +}; + +class session : public std::enable_shared_from_this<session> { +public: + session(tcp::socket in_sock, tcp::socket out_sock, std::atomic_bool& stopped, message_listener* listener) + : m_in_sock(std::move(in_sock)) + , m_out_sock(std::move(out_sock)) + , m_stopped(stopped) + , m_listener(listener) { } + + void start() { do_serve(); } + + tcp::socket &get_out_sock() { return m_out_sock; } + + void set_writable(bool writable) { + m_in_to_out_writable = writable; + m_out_to_in_writable = writable; + } + + enum direction { forward, reverse }; + +private: + void do_serve() { + do_read(forward); + do_read(reverse); + } + + void do_read(direction direction) { Review Comment: It's better not to name variable the same name as class - though it should work OK, in reality it may confuse some compilers in some cases and you will spend a long time figuring it out. ########## modules/platforms/cpp/cmake/dependencies.cmake: ########## @@ -76,6 +99,7 @@ else() fetch_dependency(uni-algo https://github.com/uni-algo/uni-algo/archive/v1.2.0.tar.gz 6e0cce94a6b45ebee7b904316df9f87f) if (${ENABLE_TESTS}) fetch_dependency(googletest https://github.com/google/googletest/archive/refs/tags/v1.14.0.tar.gz c8340a482851ef6a3fe618a082304cfc) + add_asio_dependency(https://github.com/chriskohlhoff/asio/archive/refs/tags/asio-1-36-0.tar.gz 6699ac1dea111c20d024f25e06e573db) endif() Review Comment: Why? Can you please clarify? Seems valid to me. ########## modules/platforms/cpp/tests/fake_server/proxy/asio_proxy.h: ########## @@ -0,0 +1,266 @@ +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to You under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#pragma once + +#include <atomic> +#include <iostream> +#include <map> +#include <memory> +#include <queue> +#include <thread> +#include <vector> + +#include <asio.hpp> +#include <asio/ts/internet.hpp> + +#include "message.h" +#include "message_listener.h" + +namespace ignite::proxy { + +using asio::ip::tcp; + +struct configuration { + asio::ip::port_type m_in_port; + std::string m_out_host_and_port; + message_listener* m_listener; + + configuration(asio::ip::port_type m_in_port, const std::string &m_out_host_and_port, message_listener *m_listener) + : m_in_port(m_in_port) + , m_out_host_and_port(m_out_host_and_port) + , m_listener(m_listener) { } +}; + +class session : public std::enable_shared_from_this<session> { +public: + session(tcp::socket in_sock, tcp::socket out_sock, std::atomic_bool& stopped, message_listener* listener) + : m_in_sock(std::move(in_sock)) + , m_out_sock(std::move(out_sock)) + , m_stopped(stopped) + , m_listener(listener) { } + + void start() { do_serve(); } + + tcp::socket &get_out_sock() { return m_out_sock; } + + void set_writable(bool writable) { + m_in_to_out_writable = writable; + m_out_to_in_writable = writable; + } + + enum direction { forward, reverse }; + +private: + void do_serve() { + do_read(forward); + do_read(reverse); + } + + void do_read(direction direction) { + if (m_stopped.load()) + return; + + tcp::socket &src = direction == forward ? m_in_sock : m_out_sock; + + auto self(shared_from_this()); + + src.async_read_some(asio::buffer(buf, BUFF_SIZE), + [direction, self](const asio::error_code& ec, size_t len) { + if (ec) { + if (ec == asio::error::eof) { + return; + } + throw std::runtime_error("Error while reading from socket " + ec.message()); + } + + std::queue<message> &queue = direction == forward ? self->m_in_to_out : self->m_out_to_in; + bool &writable = direction == forward ? self->m_in_to_out_writable : self->m_out_to_in_writable; + + // we have one-threaded executor no synchronization is needed + message& msg = queue.emplace(self->buf, len); + + if (self->m_listener) { + if (direction == forward) { + self->m_listener->register_out_message(msg); + } else { + self->m_listener->register_in_message(msg); + } + } + + if (writable) { // there are pending write operation on this socket + self->do_write(direction); + } + + self->do_read(direction); + }); + } + + void do_write(direction direction) { + tcp::socket &dst = direction == forward ? m_out_sock : m_in_sock; + std::queue<message> &queue = direction == forward ? m_in_to_out : m_out_to_in; + bool &writable = direction == forward ? m_in_to_out_writable : m_out_to_in_writable; + + writable = false; // protects from writing same buffer twice (from head of queue). + + auto self(shared_from_this()); + if (!queue.empty()) { + message &msg = queue.front(); Review Comment: Why not store messages as `std::shared_ptr`'s and pop them here right away? Won't it solve the issue and make `writable` flag unnecessary? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
