Repository: envoyproxy/envoy
File: source/extensions/filters/network/generic_proxy/router/upstream.h (C/C++)

Code before:
      auto local_tcp_pool_handle = tcp_pool_handle_;
      tcp_pool_handle_ = nullptr;
      local_tcp_pool_handle->cancel(Tcp::ConnectionPool::CancelPolicy::Default);
    }

    if (owned_conn_data_ != nullptr) {
      if (!close_connection) {
        // Just release the connection back to the pool.
        owned_conn_data_.reset();
        return;
      }

      ENVOY_LOG(debug, "generic proxy upstream request: close upstream connection");
      ASSERT(tcp_pool_handle_ == nullptr);

      // Clear the data first to avoid re-entering this function in the close callback.
      auto local_conn_data = std::move(owned_conn_data_);
      owned_conn_data_.reset();
      local_conn_data->connection().close(Network::ConnectionCloseType::FlushWrite);

      // NOTE: Because the local_conn_data will also clear the callbacks when
      // it is destroyed, the onEvent() callback may not be called if the connection
      // closing is delayed.
      // So we need to manually call the onConnectionClose() here.
      //
      // The onConnectionClose() implementation should ensure it is safe to be called
      // multiple times.
      onConnectionClose(Network::ConnectionEvent::LocalClose);
    }
  }
  OptRef<Network::Connection> upstreamConnection() override {
    if (owned_conn_data_ == nullptr) {
      return {};
    }
    return owned_conn_data_->connection();
  }
  Upstream::HostDescriptionConstSharedPtr upstreamHost() const override {
    return tcp_pool_data_.host();
  }

  virtual void onUpstreamSuccess() PURE;
  virtual void onUpstreamFailure(ConnectionPool::PoolFailureReason reason,
                                 absl::string_view transport_failure_reason) PURE;
  virtual void onConnectionClose(Network::ConnectionEvent) {}

protected:
  void tryInitialize() {
    if (!initialized_) {
      initialized_ = true;
      tcp_pool_handle_ = tcp_pool_data_.newConnection(*this);
    }

Review comment (suggestion):
I think it's better if we check that's indeed happening and change `should` => `would` instead or otherwise future maintainers would keep guessing whether this is the case or not.

Code after:
      auto local_tcp_pool_handle = tcp_pool_handle_;
      tcp_pool_handle_ = nullptr;
      local_tcp_pool_handle->cancel(Tcp::ConnectionPool::CancelPolicy::Default);
    }

    if (owned_conn_data_ != nullptr) {
      if (!close_connection) {
        // Just release the connection back to the pool.
        owned_conn_data_.reset();
        return;
      }

      ENVOY_LOG(debug, "generic proxy upstream request: close upstream connection");
      ASSERT(tcp_pool_handle_ == nullptr);

      // Clear the data first to avoid re-entering this function in the close callback.
      auto local_conn_data = std::move(owned_conn_data_);
      owned_conn_data_.reset();
      local_conn_data->connection().close(Network::ConnectionCloseType::FlushWrite);

      // NOTE: Because the local_conn_data will also clear the callbacks when
      // it is destroyed, the onEvent() callback may not be called if the connection
      // closing is delayed.
      // So we need to manually call the onConnectionClose() here.
      //
      // The onConnectionClose() implementation MUST ensure it is safe to be called
      // multiple times.
      onConnectionClose(Network::ConnectionEvent::LocalClose);
    }
  }
  OptRef<Network::Connection> upstreamConnection() override {
    if (owned_conn_data_ == nullptr) {
      return {};
    }
    return owned_conn_data_->connection();
  }
  Upstream::HostDescriptionConstSharedPtr upstreamHost() const override {
    return tcp_pool_data_.host();
  }

  virtual void onUpstreamSuccess() PURE;
  virtual void onUpstreamFailure(ConnectionPool::PoolFailureReason reason,
                                 absl::string_view transport_failure_reason) PURE;
  virtual void onConnectionClose(Network::ConnectionEvent) {}

protected:
  void tryInitialize() {
    if (!initialized_) {
      initialized_ = true;
      tcp_pool_handle_ = tcp_pool_data_.newConnection(*this);
    }