Skip to content

Commit bd449f0

Browse files
committed
mon/MonClient: post version request completions outside of monc_lock
dispatch() is allowed to invoke the completion object in the current thread, before control returns from dispatch(). This isn't desirable when it comes to discarding version requests in MonClient::shutdown() and MonClient::_reopen_session() because completion objects could then be invoked under monc_lock. In case of MonClient::_reopen_session() in particular, this leads to an attempt to acquire monc_lock once again in MonClient::get_version() on a retry due to monc_errc::session_reset that is converted to errc::resource_unavailable_try_again: MonClient::ms_handle_reset < takes monc_lock > MonClient::_reopen_session < invokes the completion object via dispatch() with ec == monc_errc::session_reset > Objecter::CB_Objecter_GetVersion::operator() [ ec == errc::resource_unavailable_try_again ] Objecter::_wait_for_latest_osdmap MonClient::get_version < attempts to take monc_lock in the body of the lambda > The end result is either a lockup or some form of undefined behavior. The best possible outcome here is an exception (std::system_error with "Resource deadlock avoided" error) and a successive call to std::terminate(). This is a regression introduced in commit e81d4ea ("common/async: Update `use_blocked` for newer asio"). Revert to posting version request completions for the error cases in a way that is uniform with the success case in MonClient::handle_get_version_reply(). Fixes: https://tracker.ceph.com/issues/72692 Signed-off-by: Ilya Dryomov <[email protected]>
1 parent f80ba9d commit bd449f0

File tree

1 file changed

+8
-6
lines changed

1 file changed

+8
-6
lines changed

src/mon/MonClient.cc

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -535,11 +535,11 @@ void MonClient::shutdown()
535535
monc_lock.lock();
536536
stopping = true;
537537
while (!version_requests.empty()) {
538-
asio::dispatch(
539-
asio::append(std::move(version_requests.begin()->second),
540-
make_error_code(monc_errc::shutting_down), 0, 0));
541538
ldout(cct, 20) << __func__ << " canceling and discarding version request "
542539
<< version_requests.begin()->first << dendl;
540+
asio::post(service.get_executor(),
541+
asio::append(std::move(version_requests.begin()->second),
542+
make_error_code(monc_errc::shutting_down), 0, 0));
543543
version_requests.erase(version_requests.begin());
544544
}
545545
while (!mon_commands.empty()) {
@@ -769,9 +769,11 @@ void MonClient::_reopen_session(int rank)
769769

770770
// throw out version check requests
771771
while (!version_requests.empty()) {
772-
asio::dispatch(asio::append(std::move(version_requests.begin()->second),
773-
make_error_code(monc_errc::session_reset),
774-
0, 0));
772+
ldout(cct, 20) << __func__ << " canceling and discarding version request "
773+
<< version_requests.begin()->first << dendl;
774+
asio::post(service.get_executor(),
775+
asio::append(std::move(version_requests.begin()->second),
776+
make_error_code(monc_errc::session_reset), 0, 0));
775777
version_requests.erase(version_requests.begin());
776778
}
777779

0 commit comments

Comments
 (0)