[SERVER-72605] Ensure and document thread-safety semantics for `Session::end()` Created: 06/Jan/23  Updated: 12/Dec/23  Resolved: 12/Dec/23

Status: Closed
Project: Core Server
Component/s: Internal Code
Affects Version/s: None
Fix Version/s: 7.3.0-rc0

Type: Task Priority: Major - P3
Reporter: Amirsaman Memaripour Assignee: James Bronsted
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
Related
related to SERVER-84083 Document thread-safety semantics for ... Needs Scheduling
related to SERVER-83933 Move AsioSession ingress initializati... Open
Assigned Teams:
Service Arch
Backwards Compatibility: Fully Compatible
Sprint: 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
Participants:
Linked BF Score: 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.



 Comments   
Comment by Githook User [ 12/Dec/23 ]

Author:

{'name': 'James Bronsted', 'email': 'james.bronsted@mongodb.com', 'username': 'jpbronsted'}

Message: SERVER-72605 guard accesses to the SSL socket that could come from multiple threads with a mutex

GitOrigin-RevId: 63393f8678fcd31db056d43324edeb766d43f612
Branch: master
https://github.com/mongodb/mongo/commit/b6c7f545b4d5bd25e51c3783243ca7e3b55c3103

Comment by Matt Diener (Inactive) [ 27/Apr/23 ]

Note: I suspect this might be resolved if the Session doesn't manifest until after the handshake is complete. As in: https://github.com/10gen/mongo/pull/12404 (see CommonAsioSession::initForIngress).

Generated at Thu Feb 08 06:22:17 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.