Uploaded image for project: 'Core Server'
  1. Core Server
  2. SERVER-72605

Ensure and document thread-safety semantics for `Session::end()`

    • Type: Icon: Task Task
    • Resolution: Fixed
    • Priority: Icon: Major - P3 Major - P3
    • 7.3.0-rc0
    • Affects Version/s: None
    • Component/s: Internal Code
    • 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

      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.

            Assignee:
            james.bronsted@mongodb.com James Bronsted
            Reporter:
            amirsaman.memaripour@mongodb.com Amirsaman Memaripour
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: