Skip to content

Commit

Permalink
tests: client: simplify HTTP-level error handling
Browse files Browse the repository at this point in the history 
Since commit ca1ad2c, the HTTP client
which is used in the test for one-shot HTTP queries contained some code
which deferred HTTP error handling. The reason why this code was needed
is that when the request failed on the HTTP level, and if an exception
is thrown (that's what the `client.on_error(...)` call is doing), then
the usual rules of stack unwinding took effect, which meant that the
entire `client` variable went out of scope. Its destructor would then
try to call the request's `on_close(...)`, which was trying to shut down
the HTTP client, but that one was already being destroyed:

  ERROR: AddressSanitizer: stack-use-after-scope on address 0x7f03e8cc60d0 at pc 0x565524c6a158 bp 0x7ffe0c7ab2d0 sp 0x7ffe0c7ab2c8
  READ of size 8 at 0x7f03e8cc60d0 thread T0
    #0 0x565524c6a157 in std::__shared_ptr<nghttp2::asio_http2::client::session, (__gnu_cxx::_Lock_policy)2>::get() const /nix/store/j00nb8s5mwaxgi77h21i1ycb91yxxqck-gcc-13.2.0/include/c++/13.2.0/bits/shared_ptr_base.h:1666:16
    #1 0x565524c6a157 in std::__shared_ptr_access<nghttp2::asio_http2::client::session, (__gnu_cxx::_Lock_policy)2, false, false>::_M_get() const /nix/store/j00nb8s5mwaxgi77h21i1ycb91yxxqck-gcc-13.2.0/include/c++/13.2.0/bits/shared_ptr_base.h:1363:66
    #2 0x565524c6a157 in std::__shared_ptr_access<nghttp2::asio_http2::client::session, (__gnu_cxx::_Lock_policy)2, false, false>::operator->() const /nix/store/j00nb8s5mwaxgi77h21i1ycb91yxxqck-gcc-13.2.0/include/c++/13.2.0/bits/shared_ptr_base.h:1357:9
    #3 0x565524c6a157 in auto auto Response clientRequest<char const*, char const*>(char const*, char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>>> const&, boost::posix_time::time_duration)::'lambda'(char const*)::operator()<boost::asio::ip::basic_resolver_iterator<boost::asio::ip::tcp>>(char const*) const::'lambda'(char const*)::operator()<unsigned int>(char const*) const CzechLight/rousette/tests/aux-utils.h:155:17
    #4 0x565524c6a157 in char const* std::__invoke_impl<void, auto Response clientRequest<char const*, char const*>(char const*, char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>>> const&, boost::posix_time::time_duration)::'lambda'(char const*)::operator()<boost::asio::ip::basic_resolver_iterator<boost::asio::ip::tcp>>(char const*) const::'lambda'(char const*)&, unsigned int>(std::__invoke_other, char const*&&, unsigned int&&) /nix/store/j00nb8s5mwaxgi77h21i1ycb91yxxqck-gcc-13.2.0/include/c++/13.2.0/bits/invoke.h:61:14
    #5 0x565524c6a157 in std::enable_if<is_invocable_r_v<char const*, char const*, unsigned int>, char const*>::type std::__invoke_r<void, auto Response clientRequest<char const*, char const*>(char const*, char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>>> const&, boost::posix_time::time_duration)::'lambda'(char const*)::operator()<boost::asio::ip::basic_resolver_iterator<boost::asio::ip::tcp>>(char const*) const::'lambda'(char const*)&, unsigned int>(char const*&&, unsigned int&&) /nix/store/j00nb8s5mwaxgi77h21i1ycb91yxxqck-gcc-13.2.0/include/c++/13.2.0/bits/invoke.h:111:2
    #6 0x565524c6a157 in std::_Function_handler<void (unsigned int), auto Response clientRequest<char const*, char const*>(char const*, char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>>> const&, boost::posix_time::time_duration)::'lambda'(char const*)::operator()<boost::asio::ip::basic_resolver_iterator<boost::asio::ip::tcp>>(char const*) const::'lambda'(char const*)>::_M_invoke(std::_Any_data const&, unsigned int&&) /nix/store/j00nb8s5mwaxgi77h21i1ycb91yxxqck-gcc-13.2.0/include/c++/13.2.0/bits/std_function.h:290:9
    #7 0x7f03ecc55683 in std::function<void (unsigned int)>::operator()(unsigned int) const /nix/store/j00nb8s5mwaxgi77h21i1ycb91yxxqck-gcc-13.2.0/include/c++/13.2.0/bits/std_function.h:591:9
    #8 0x7f03ecc55683 in nghttp2::asio_http2::client::request_impl::call_on_close(unsigned int) github/nghttp2/nghttp2-asio/lib/asio_client_request_impl.cc:67:5
    #9 0x7f03ecbf6daa in nghttp2::asio_http2::client::session_impl::~session_impl() github/nghttp2/nghttp2-asio/lib/asio_client_session_impl.cc:63:9
    #10 0x565524c40cd8 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release_last_use() /nix/store/j00nb8s5mwaxgi77h21i1ycb91yxxqck-gcc-13.2.0/include/c++/13.2.0/bits/shared_ptr_base.h:175:2
    #11 0x7f03ecc0854e in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() /nix/store/j00nb8s5mwaxgi77h21i1ycb91yxxqck-gcc-13.2.0/include/c++/13.2.0/bits/shared_ptr_base.h:1071:11
    #12 0x7f03ecc0854e in std::__shared_ptr<nghttp2::asio_http2::client::session_impl, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() /nix/store/j00nb8s5mwaxgi77h21i1ycb91yxxqck-gcc-13.2.0/include/c++/13.2.0/bits/shared_ptr_base.h:1524:31
    #13 0x7f03ecc0854e in nghttp2::asio_http2::client::session_impl::do_read()::$_0::~$_0() github/nghttp2/nghttp2-asio/lib/asio_client_session_impl.cc:629:15
    #14 0x7f03ecc0854e in std::_Function_base::_Base_manager<nghttp2::asio_http2::client::session_impl::do_read()::$_0>::_M_destroy(std::_Any_data&, std::integral_constant<bool, false>) /nix/store/j00nb8s5mwaxgi77h21i1ycb91yxxqck-gcc-13.2.0/include/c++/13.2.0/bits/std_function.h:175:4
    #15 0x7f03ecc0854e in std::_Function_base::_Base_manager<nghttp2::asio_http2::client::session_impl::do_read()::$_0>::_M_manager(std::_Any_data&, std::_Any_data const&, std::_Manager_operation) /nix/store/j00nb8s5mwaxgi77h21i1ycb91yxxqck-gcc-13.2.0/include/c++/13.2.0/bits/std_function.h:203:8
    #16 0x7f03ecc0854e in std::_Function_handler<void (boost::system::error_code const&, unsigned long), nghttp2::asio_http2::client::session_impl::do_read()::$_0>::_M_manager(std::_Any_data&, std::_Any_data const&, std::_Manager_operation) /nix/store/j00nb8s5mwaxgi77h21i1ycb91yxxqck-gcc-13.2.0/include/c++/13.2.0/bits/std_function.h:282:6
    #17 0x7f03ecc1d18c in std::_Function_base::~_Function_base() /nix/store/j00nb8s5mwaxgi77h21i1ycb91yxxqck-gcc-13.2.0/include/c++/13.2.0/bits/std_function.h:244:2
    #18 0x7f03ecc1d18c in boost::asio::detail::binder2<std::function<void (boost::system::error_code const&, unsigned long)>, boost::system::error_code, unsigned long>::~binder2() /home/jkt/work/prog/_build/czechlight-clang17-asan-ubsan/nghttp2-asio/include/boost/asio/detail/bind_handler.hpp:252:7
    #19 0x7f03ecc1d18c in boost::asio::detail::reactive_socket_recv_op<boost::asio::mutable_buffers_1, std::function<void (boost::system::error_code const&, unsigned long)>, boost::asio::any_io_executor>::do_complete(void*, boost::asio::detail::scheduler_operation*, boost::system::error_code const&, unsigned long) /home/jkt/work/prog/_build/czechlight-clang17-asan-ubsan/nghttp2-asio/include/boost/asio/detail/reactive_socket_recv_op.hpp:155:3
    #20 0x565524c5b55e in boost::asio::detail::scheduler_operation::destroy() /home/jkt/work/prog/_build/czechlight-clang17-asan-ubsan/rousette/include/boost/asio/detail/scheduler_operation.hpp:45:5
    #21 0x565524c5b55e in void boost::asio::detail::op_queue_access::destroy<boost::asio::detail::scheduler_operation>(boost::asio::detail::scheduler_operation*) /home/jkt/work/prog/_build/czechlight-clang17-asan-ubsan/rousette/include/boost/asio/detail/op_queue.hpp:47:8
    #22 0x565524c5b55e in boost::asio::detail::op_queue<boost::asio::detail::scheduler_operation>::~op_queue() /home/jkt/work/prog/_build/czechlight-clang17-asan-ubsan/rousette/include/boost/asio/detail/op_queue.hpp:81:7
    #23 0x565524c5b55e in boost::asio::detail::scheduler::abandon_operations(boost::asio::detail::op_queue<boost::asio::detail::scheduler_operation>&) /home/jkt/work/prog/_build/czechlight-clang17-asan-ubsan/rousette/include/boost/asio/detail/impl/scheduler.ipp:447:1
    #24 0x565524c55f1f in boost::asio::detail::epoll_reactor::shutdown() /home/jkt/work/prog/_build/czechlight-clang17-asan-ubsan/rousette/include/boost/asio/detail/impl/epoll_reactor.ipp:92:14
    #25 0x565524c63f95 in boost::asio::detail::service_registry::shutdown_services() /home/jkt/work/prog/_build/czechlight-clang17-asan-ubsan/rousette/include/boost/asio/detail/impl/service_registry.ipp:44:14
    #26 0x565524c4a441 in boost::asio::execution_context::shutdown() /home/jkt/work/prog/_build/czechlight-clang17-asan-ubsan/rousette/include/boost/asio/impl/execution_context.ipp:41:22
    #27 0x565524c4a441 in boost::asio::io_context::~io_context() /home/jkt/work/prog/_build/czechlight-clang17-asan-ubsan/rousette/include/boost/asio/impl/io_context.ipp:58:3
    #28 0x565524c426c1 in Response clientRequest<char const*, char const*>(char const*, char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>>> const&, boost::posix_time::time_duration) CzechLight/rousette/tests/aux-utils.h:165:1
    #29 0x565524c34d88 in DOCTEST_ANON_FUNC_4() CzechLight/rousette/tests/restconf-nacm.cpp:254:13
    #30 0x565524d65c92 in doctest::Context::run() /home/jkt/work/prog/_build/czechlight-clang17-asan-ubsan/target/include/doctest/doctest.h:7007:21
    #31 0x565524d6ae96 in main /home/jkt/work/prog/_build/czechlight-clang17-asan-ubsan/target/include/doctest/doctest.h:7085:71
    #32 0x7f03ebd5d0cd in __libc_start_call_main (/nix/store/7jiqcrg061xi5clniy7z5pvkc4jiaqav-glibc-2.38-27/lib/libc.so.6+0x280cd) (BuildId: fd8eb9ded07dda31c46fd63a5351b53bac6a4f1d)
    #33 0x7f03ebd5d188 in __libc_start_main@GLIBC_2.2.5 (/nix/store/7jiqcrg061xi5clniy7z5pvkc4jiaqav-glibc-2.38-27/lib/libc.so.6+0x28188) (BuildId: fd8eb9ded07dda31c46fd63a5351b53bac6a4f1d)
    #34 0x565524aeed04 in _start (/home/jkt/work/prog/_build/czechlight-clang17-asan-ubsan/rousette/test-restconf-nacm+0x111d04)

I suspected that this is a bug in nghttp2-asio, but this turned out to
be just a plain bug in interaction between C++'s memory management and
ASIO, and the cure is to use weak_ptr which "sees" that the original
object is already gone.

Change-Id: I72ac8c5618ac339fcdd800398713667b20c17f4c
  • Loading branch information
@jktjkt @peckato1
jktjkt authored and peckato1 committed Jun 18, 2024
1 parent 139c991 commit 27114e2
Showing 1 changed file with 10 additions and 13 deletions.
23 changes: 10 additions & 13 deletions tests/aux-utils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,44 +125,41 @@ Response clientRequest(auto method,
const boost::posix_time::time_duration timeout=boost::posix_time::seconds(3))
{
boost::asio::io_service io_service;
ng_client::session client(io_service, SERVER_ADDRESS, SERVER_PORT);
auto client = std::make_shared<ng_client::session>(io_service, SERVER_ADDRESS, SERVER_PORT);

client.read_timeout(timeout);
client->read_timeout(timeout);

std::ostringstream oss;
ng::header_map resHeaders;
int statusCode;
std::optional<std::string> clientError;

client.on_connect([&](auto) {
client->on_connect([&](auto) {
boost::system::error_code ec;

ng::header_map reqHeaders;
for (const auto& [name, value] : headers) {
reqHeaders.insert({name, {value, false}});
}

auto req = client.submit(ec, method, SERVER_ADDRESS_AND_PORT + uri, data, reqHeaders);
auto req = client->submit(ec, method, SERVER_ADDRESS_AND_PORT + uri, data, reqHeaders);
req->on_response([&](const ng_client::response& res) {
res.on_data([&oss](const uint8_t* data, std::size_t len) {
oss.write(reinterpret_cast<const char*>(data), len);
});
statusCode = res.status_code();
resHeaders = res.header();
});
req->on_close([&client](auto) {
client.shutdown();
req->on_close([maybeClient = std::weak_ptr<ng_client::session>{client}](auto) {
if (auto client = maybeClient.lock()) {
client->shutdown();
}
});
});
client.on_error([&clientError](const boost::system::error_code& ec) {
clientError = ec.message();
client->on_error([](const boost::system::error_code& ec) {
throw std::runtime_error{"HTTP client error: " + ec.message()};
});
io_service.run();

if (clientError) {
throw std::runtime_error{"HTTP client error: " + *clientError};
}

return {statusCode, resHeaders, oss.str()};
}

Expand Down

0 comments on commit 27114e2

Please sign in to comment.