Skip to content

Commit

Permalink
crimson/admin: do not reset connected_sock before closing
Browse files Browse the repository at this point in the history
* no need to discard_result(). as `output_stream::close()` returns an
  empty future<> already
* free the connected socket after the background task finishes, because:

we should not free the connected socket before the promise referencing it is fulfilled.

otherwise we have error messages from ASan, like

==287182==ERROR: AddressSanitizer: heap-use-after-free on address 0x611000019aa0 at pc 0x55e2ae2de882 bp 0x7fff7e2bf080 sp 0x7fff7e2bf078
READ of size 8 at 0x611000019aa0 thread T0
    #0 0x55e2ae2de881 in seastar::reactor_backend_aio::await_events(int, __sigset_t const*) ../src/seastar/src/core/reactor_backend.cc:396
    #1 0x55e2ae2dfb59 in seastar::reactor_backend_aio::reap_kernel_completions() ../src/seastar/src/core/reactor_backend.cc:428
    #2 0x55e2adbea397 in seastar::reactor::reap_kernel_completions_pollfn::poll() (/var/ssd/ceph/build/bin/crimson-osd+0x155e9397)
    #3 0x55e2adaec6d0 in seastar::reactor::poll_once() ../src/seastar/src/core/reactor.cc:2789
    #4 0x55e2adae7cf7 in operator() ../src/seastar/src/core/reactor.cc:2687
    #5 0x55e2adb7c595 in __invoke_impl<bool, seastar::reactor::run()::<lambda()>&> /usr/include/c++/10/bits/invoke.h:60
    #6 0x55e2adb699b0 in __invoke_r<bool, seastar::reactor::run()::<lambda()>&> /usr/include/c++/10/bits/invoke.h:113
    #7 0x55e2adb50222 in _M_invoke /usr/include/c++/10/bits/std_function.h:291
    #8 0x55e2adc2ba00 in std::function<bool ()>::operator()() const /usr/include/c++/10/bits/std_function.h:622
    #9 0x55e2adaea491 in seastar::reactor::run() ../src/seastar/src/core/reactor.cc:2713
    #10 0x55e2ad98f1c7 in seastar::app_template::run_deprecated(int, char**, std::function<void ()>&&) ../src/seastar/src/core/app-template.cc:199
    #11 0x55e2a9e57538 in main ../src/crimson/osd/main.cc:148
    #12 0x7fae7f20de0a in __libc_start_main ../csu/libc-start.c:308
    #13 0x55e2a9d431e9 in _start (/var/ssd/ceph/build/bin/crimson-osd+0x117421e9)

0x611000019aa0 is located 96 bytes inside of 240-byte region [0x611000019a40,0x611000019b30)
freed by thread T0 here:
    #0 0x7fae80a4e487 in operator delete(void*, unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.6+0xac487)
    #1 0x55e2ae302a0a in seastar::aio_pollable_fd_state::~aio_pollable_fd_state() ../src/seastar/src/core/reactor_backend.cc:458
    #2 0x55e2ae2e1059 in seastar::reactor_backend_aio::forget(seastar::pollable_fd_state&) ../src/seastar/src/core/reactor_backend.cc:524
    #3 0x55e2adab9b9a in seastar::pollable_fd_state::forget() ../src/seastar/src/core/reactor.cc:1396
    #4 0x55e2adab9d05 in seastar::intrusive_ptr_release(seastar::pollable_fd_state*) ../src/seastar/src/core/reactor.cc:1401
    #5 0x55e2ace1b72b in boost::intrusive_ptr<seastar::pollable_fd_state>::~intrusive_ptr() /opt/ceph/include/boost/smart_ptr/intrusive_ptr.hpp:98
    #6 0x55e2ace115a5 in seastar::pollable_fd::~pollable_fd() ../src/seastar/include/seastar/core/internal/pollable_fd.hh:109
    #7 0x55e2ae0ed35c in seastar::net::posix_server_socket_impl::~posix_server_socket_impl() ../src/seastar/include/seastar/net/posix-stack.hh:161
    #8 0x55e2ae0ed3cf in seastar::net::posix_server_socket_impl::~posix_server_socket_impl() ../src/seastar/include/seastar/net/posix-stack.hh:161
    #9 0x55e2ae0ed943 in std::default_delete<seastar::net::api_v2::server_socket_impl>::operator()(seastar::net::api_v2::server_socket_impl*) const /usr/include/c++/10/bits/unique_ptr.h:81
    #10 0x55e2ae0db357 in std::unique_ptr<seastar::net::api_v2::server_socket_impl, std::default_delete<seastar::net::api_v2::server_socket_impl> >::~unique_ptr()
	/usr/include/c++/10/bits/unique_ptr.h:357    #11 0x55e2ae1438b7 in seastar::api_v2::server_socket::~server_socket() ../src/seastar/src/net/stack.cc:195
    #12 0x55e2aa1c7656 in std::_Optional_payload_base<seastar::api_v2::server_socket>::_M_destroy() /usr/include/c++/10/optional:260
    #13 0x55e2aa16c84b in std::_Optional_payload_base<seastar::api_v2::server_socket>::_M_reset() /usr/include/c++/10/optional:280
    #14 0x55e2ac24b2b7 in std::_Optional_base_impl<seastar::api_v2::server_socket, std::_Optional_base<seastar::api_v2::server_socket, false, false> >::_M_reset() /usr/include/c++/10/optional:432
    #15 0x55e2ac23f37b in std::optional<seastar::api_v2::server_socket>::reset() /usr/include/c++/10/optional:975
    #16 0x55e2ac21a2e7 in crimson::admin::AdminSocket::stop() ../src/crimson/admin/admin_socket.cc:265
    #17 0x55e2aa099825 in operator() ../src/crimson/osd/osd.cc:450
    #18 0x55e2aa0d4e3e in apply ../src/seastar/include/seastar/core/apply.hh:36

Signed-off-by: Kefu Chai <kchai@redhat.com>
  • Loading branch information
tchaikov committed Mar 23, 2020
1 parent ec362f4 commit 1ce152f
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 10 deletions.
19 changes: 9 additions & 10 deletions src/crimson/admin/admin_socket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,7 @@ seastar::future<> AdminSocket::handle_client(seastar::input_stream<char>& in,
return in.close();
}).handle_exception([](auto ep) {
logger().debug("exception on {}: {}", __func__, ep);
return seastar::make_ready_future<>();
}).discard_result();
});
}

seastar::future<> AdminSocket::start(const std::string& path)
Expand All @@ -236,7 +235,7 @@ seastar::future<> AdminSocket::start(const std::string& path)
auto sock_path = seastar::socket_address{ seastar::unix_domain_addr{ path } };
server_sock = seastar::engine().listen(sock_path);
// listen in background
std::ignore = seastar::do_until(
task = seastar::do_until(
[this] { return stop_gate.is_closed(); },
[this] {
return seastar::with_gate(stop_gate, [this] {
Expand All @@ -257,11 +256,7 @@ seastar::future<> AdminSocket::start(const std::string& path)
}
});
});
}).then([] {
logger().debug("AdminSocket::init(): admin-sock thread terminated");
return seastar::now();
});

return seastar::make_ready_future<>();
}

Expand All @@ -271,13 +266,17 @@ seastar::future<> AdminSocket::stop()
return seastar::now();
}
server_sock->abort_accept();
server_sock.reset();
if (connected_sock) {
connected_sock->shutdown_input();
connected_sock->shutdown_output();
connected_sock.reset();
}
return stop_gate.close();
return stop_gate.close().then([this] {
assert(task.has_value());
return task->then([] {
logger().info("AdminSocket: stopped");
return seastar::now();
});
});
}

/////////////////////////////////////////
Expand Down
1 change: 1 addition & 0 deletions src/crimson/admin/admin_socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ class AdminSocket : public seastar::enable_lw_shared_from_this<AdminSocket> {
seastar::future<tell_result_t> execute_command(const std::vector<std::string>& cmd,
ceph::bufferlist&& buf);

std::optional<seastar::future<>> task;
std::optional<seastar::server_socket> server_sock;
std::optional<seastar::connected_socket> connected_sock;

Expand Down

0 comments on commit 1ce152f

Please sign in to comment.