Details
-
Task
-
Resolution: Fixed
-
Major - P3
-
None
-
None
-
Service Arch
-
Fully Compatible
-
Service Arch 2023-01-23, Service Arch 2023-02-06, Service Arch 2023-02-20, Service Arch 2023-03-06, Service Arch 2023-10-16, Service Arch 2023-10-30, Service Arch 2023-11-13, Service Arch 2023-11-27, Service Arch 2023-12-11, Service Arch 2023-12-25
-
120
Description
Other threads may use Session::end() to interrupt ongoing networking and close the session. For example, ServiceEntryPointImpl (responsible for managing ingress sessions) calls into SessionWorkflow::terminate as part of its shutdown process. That will result in calling Session::end:
void SessionWorkflow::Impl::terminate() { |
if (_isTerminated.swap(true)) |
return; |
session()->end();
|
}
|
...
|
void AsioTransportLayer::AsioSession::end() { |
if (getSocket().is_open()) { |
std::error_code ec;
|
getSocket().shutdown(GenericSocket::shutdown_both, ec);
|
if ((ec) && (ec != asio::error::not_connected)) { |
LOGV2_ERROR(23841,
|
"Error shutting down socket: {error}", |
"Error shutting down socket", |
"error"_attr = ec.message()); |
}
|
}
|
}
|
...
|
auto AsioTransportLayer::AsioSession::getSocket() -> GenericSocket& {
|
#ifdef MONGO_CONFIG_SSL
|
if (_sslSocket) { |
return static_cast<GenericSocket&>(_sslSocket->lowest_layer()); |
}
|
#endif
|
return _socket; |
}
|
As shown above, and when SSL is enabled and we use AsioSession, terminating a session from another thread requires accessing _sslSocket and _socket members of AsioSession. This may happen when the thread owning the Session is performing an SSL handshake (as part of the initial read on the socket):
std::unique_ptr<SessionWorkflow::Impl::WorkItem> SessionWorkflow::Impl::_receiveRequest() {
|
try { |
auto msg = uassertStatusOK([&] {
|
MONGO_IDLE_THREAD_BLOCK;
|
return session()->sourceMessage(); |
}());
|
invariant(!msg.empty());
|
return std::make_unique<WorkItem>(this, std::move(msg)); |
} catch (const DBException& ex) { ... } |
}
|
...
|
StatusWith<Message> AsioTransportLayer::AsioSession::sourceMessage() noexcept try { |
ensureSync();
|
return sourceMessageImpl().getNoThrow(); |
} catch (const DBException& ex) { |
return ex.toStatus(); |
}
|
The current implementation for the handshake performs the following, creating a potential read after write data-race on the _sslSocket member:
template <typename MutableBufferSequence> |
Future<bool> AsioTransportLayer::AsioSession::maybeHandshakeSSLForIngress( |
const MutableBufferSequence& buffer) { |
...
|
_sslSocket.emplace(std::move(_socket), *_sslContext->ingress, ""); |
...
|
}
|
The goal of this ticket is to ensure terminating sessions is a thread-safe operation, regardless of the state of the Session object. For context, SERVER-62970 addressed a similar issue for SSL handshake on egress connections.
Attachments
Issue Links
- related to
-
SERVER-84083 Document thread-safety semantics for ASIOSession
-
- Needs Scheduling
-
-
SERVER-83933 Move AsioSession ingress initialization steps before startSession
-
- Open
-