Skip to content

Commit 569e17b

Browse files
committed
test_utils: reset logger ostream pointer in tee logger
The tee_wrapper helper was being used to set the ostream pointer for global logger to be an ostream instance that lived only on the stack, and not resetting it to something global like std::cerr after the test finished. This resulted in the following segfault in other tests that ran after the change but didn't make their own change to ostream pointer: ``` #0 0x7f875b0bbc1f in std::__1::basic_ostream<char, std::__1::char_traits<char> >::sentry::sentry(std::__1::basic_ostream<char, std::__1::char_traits<char> >&) (/vectorized/lib/libc++.so.1+0x65c1f) #1 0x5597c3979afc in std::__1::basic_ostream<char, std::__1::char_traits<char> >& std::__1::__put_character_sequence<char, std::__1::char_traits<char> >(std::__1::basic_ostream<char, std::__1::char_traits<char> >&, char const*, unsigned long) /vectorized/llvm/bin/../include/c++/v1/ostream:722:57 #2 0x5597c4079e44 in std::__1::basic_ostream<char, std::__1::char_traits<char> >& std::__1::operator<<<char, std::__1::char_traits<char> >(std::__1::basic_ostream<char, std::__1::char_traits<char> >&, std::__1::basic_string_view<char, std::__1::char_traits<char> >) /vectorized/llvm/bin/../include/c++/v1/ostream:1057:12 #3 0x5597c602c9f3 in seastar::logger::do_log(seastar::log_level, seastar::logger::log_writer&) /v/build/v_deps_build/seastar-prefix/src/seastar/src/util/log.cc:342:15 redpanda-data#4 0x5597c3c4ad20 in void seastar::logger::log<char const*, int, seastar::basic_sstring<char, unsigned int, 15u, true>&>(seastar::log_level, seastar::logger::format_info, char const*&&, int&&, seastar::basic_sstring<char, unsigned int, 15u, true>&) /vectorized/include/seastar/util/log.hh:231:17 redpanda-data#5 0x5597c3f23c51 in void seastar::logger::trace<char const*, int, seastar::basic_sstring<char, unsigned int, 15u, true>&>(seastar::logger::format_info, char const*&&, int&&, seastar::basic_sstring<char, unsigned int, 15u, true>&) /vectorized/include/seastar/util/log.hh:385:9 redpanda-data#6 0x5597c3f14b95 in cloud_roles::signature_v4::sign_header(boost::beast::http::header<true, boost::beast::http::basic_fields<std::__1::allocator<char> > >&, std::__1::basic_string_view<char, std::__1::char_traits<char> >) const /var/lib/buildkite-agent/builds/buildkite-amd64-builders-i-00e717c5e253e2891-1/redpanda/vtools/src/v/cloud_roles/signature.cc:382:5 redpanda-data#7 0x5597c3bf1dfb in cloud_roles::apply_aws_credentials::add_auth(boost::beast::http::header<true, boost::beast::http::basic_fields<std::__1::allocator<char> > >&) const /var/lib/buildkite-agent/builds/buildkite-amd64-builders-i-00e717c5e253e2891-1/redpanda/vtools/src/v/cloud_roles/apply_aws_credentials.cc:83:23 redpanda-data#8 0x5597c3b8f88d in cloud_roles::apply_credentials::add_auth(boost::beast::http::header<true, boost::beast::http::basic_fields<std::__1::allocator<char> > >&) const /var/lib/buildkite-agent/builds/buildkite-amd64-builders-i-00e717c5e253e2891-1/redpanda/vtools/src/v/cloud_roles/apply_credentials.h:47:23 redpanda-data#9 0x5597c3b8c578 in test_aws_headers::test_method() /var/lib/buildkite-agent/builds/buildkite-amd64-builders-i-00e717c5e253e2891-1/redpanda/vtools/src/v/cloud_roles/tests/credential_tests.cc:50:17 ``` This change resets the ostream pointer in the wrapper destructor. Signed-off-by: Noah Watkins <[email protected]>
1 parent 2458519 commit 569e17b

File tree

2 files changed

+22
-11
lines changed

2 files changed

+22
-11
lines changed

src/v/cloud_roles/tests/fetch_credentials_tests.cc

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -341,8 +341,7 @@ FIXTURE_TEST(test_short_lived_sts_credentials, http_imposter_fixture) {
341341
}
342342

343343
FIXTURE_TEST(test_client_closed_on_error, http_imposter_fixture) {
344-
tee_wrapper wrapper;
345-
cloud_roles::clrl_log.set_ostream(wrapper.stream);
344+
tee_wrapper wrapper(cloud_roles::clrl_log);
346345

347346
fail_request_if(
348347
[](const auto&) { return true; },
@@ -379,8 +378,7 @@ FIXTURE_TEST(test_handle_temporary_timeout, http_imposter_fixture) {
379378
// refresh operation will attempt to retry. In order not to expose the retry
380379
// counter or make similar changes to the class just for testing, this test
381380
// scans the log for the message emitted when a ss::timed_out_error is seen.
382-
tee_wrapper wrapper;
383-
cloud_roles::clrl_log.set_ostream(wrapper.stream);
381+
tee_wrapper wrapper(cloud_roles::clrl_log);
384382
ss::abort_source as;
385383
std::optional<cloud_roles::credentials> c;
386384

@@ -408,8 +406,7 @@ FIXTURE_TEST(test_handle_temporary_timeout, http_imposter_fixture) {
408406
}
409407

410408
FIXTURE_TEST(test_handle_bad_response, http_imposter_fixture) {
411-
tee_wrapper wrapper;
412-
cloud_roles::clrl_log.set_ostream(wrapper.stream);
409+
tee_wrapper wrapper(cloud_roles::clrl_log);
413410

414411
fail_request_if(
415412
[](const auto&) { return true; },
@@ -449,8 +446,7 @@ FIXTURE_TEST(test_intermittent_error, http_imposter_fixture) {
449446
// successful request. The refresh credentials object should retry after the
450447
// first failure.
451448

452-
tee_wrapper wrapper;
453-
cloud_roles::clrl_log.set_ostream(wrapper.stream);
449+
tee_wrapper wrapper(cloud_roles::clrl_log);
454450

455451
when()
456452
.request(cloud_role_tests::aws_role_query_url)

src/v/test_utils/tee_log.h

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,29 +11,43 @@
1111

1212
#pragma once
1313

14+
#include <seastar/util/log.hh>
15+
1416
#include <boost/iostreams/stream.hpp>
1517
#include <boost/iostreams/tee.hpp>
1618

1719
#include <iostream>
1820
#include <sstream>
1921

22+
/*
23+
* A utility for capturing seastar::logger output.
24+
*
25+
* TODO: ideally this introspects the installed logger so that the proper logger
26+
* could be reinstalled in the destructor. The default logger for seastar is
27+
* std::cerr which is used here. As long as this utility is only used in unit
28+
* tests then this default behavior should be sufficient.
29+
*/
2030
struct tee_wrapper {
2131
using device_t
2232
= boost::iostreams::tee_device<std::ostream, std::stringstream>;
2333

2434
using stream_t = boost::iostreams::stream<device_t>;
2535

26-
tee_wrapper()
36+
explicit tee_wrapper(seastar::logger& logger)
2737
: sstr{}
28-
, device{std::cout, sstr}
29-
, stream{device} {}
38+
, device{std::cerr, sstr}
39+
, stream{device}
40+
, logger(logger) {
41+
logger.set_ostream(stream);
42+
}
3043

3144
tee_wrapper(const tee_wrapper&) = delete;
3245
tee_wrapper& operator=(const tee_wrapper&) = delete;
3346
tee_wrapper(tee_wrapper&&) = delete;
3447
tee_wrapper& operator=(tee_wrapper&&) = delete;
3548

3649
~tee_wrapper() {
50+
logger.set_ostream(std::cerr);
3751
if (stream.is_open()) {
3852
stream->flush();
3953
stream->close();
@@ -45,4 +59,5 @@ struct tee_wrapper {
4559
std::stringstream sstr;
4660
device_t device;
4761
stream_t stream;
62+
seastar::logger& logger;
4863
};

0 commit comments

Comments
 (0)