A better with_timeout feature?

[niekbouman]

A problem with Seastar's current with_timeout API is that the future becomes orphaned upon timeout.
An alternative implementation, which also features optional abort, is the following.
@avikivity Would this code be welcomed as a PR?

//! Await a future with (optionally abortable) timeout, without orphaning the future
/*!
 * Although Seastar provides 'with_timeout', its problem is that the future provided to the function
 * gets orphaned upon timeout, which is often undesirable.
 *
 * This function always returns a pair consisting of the original future and a bool which is true in case of time out.
 * Important: in case time-out is true, the future might actually also have resolved (nearly) simultaneously.
 */
template <seastar::coroutine::future_type Future, typename Clock = seastar::steady_clock_type>
seastar::future<std::pair<Future, bool>> as_future_with_timeout(typename Clock::duration timeout, Future f,
                                                                seastar::abort_source* as = nullptr)
{
    seastar::abort_source timeout_as;
    seastar::future<> timeout_fut = seastar::sleep_abortable(timeout, timeout_as);
    seastar::optimized_optional<seastar::abort_source::subscription> subscription;
    if (as) {
        subscription = as->subscribe([&]() noexcept { timeout_as.request_abort(); });
        if (!subscription) {
            // if abort_source was already canceled, abort immediately
            subscription->on_abort(std::nullopt);
        }
    }
    auto result = co_await seastar::when_any(std::move(f), std::move(timeout_fut));
    std::tie(f, timeout_fut) = std::move(result.futures);
    if (result.index == 1) {
        if (timeout_fut.failed()) { // aborted
            timeout_fut.ignore_ready_future();
            co_return std::make_pair(std::move(f), false);
        }
        else { // timeout
            timeout_fut.ignore_ready_future();
            co_return std::make_pair(std::move(f), true);
        }
    }
    else { // f resolved
        timeout_as.request_abort();
        auto f_awaited = co_await seastar::coroutine::as_future(std::move(timeout_fut));
        f_awaited.ignore_ready_future();
        co_return std::make_pair(std::move(f), false);
    }
}

[avikivity]

Can you show how it's intended to be used?

[niekbouman]

Something like:

while(some_cond) {

    // ...

            // trigger the TLS handshake
            auto [cert_chain_f, timed_out] = co_await utils::as_future_with_timeout(
                opts.tls_handshake_timeout, seastar::tls::get_peer_certificate_chain(stream->socket), &abort_source);

            if (abort_source.abort_requested() || (timed_out && !cert_chain_f.available())) {

                if (timed_out && !abort_source.abort_requested()) {
                    logger.warn("TLS handshake timed out on connection with peer {}. Ignoring connection and "
                                "retrying...", peer.hostname);
                }

                stream->shutdown_input();  // this should trigger an exception
                stream->shutdown_output(); // in the TLS handshake
                auto f = co_await seastar::coroutine::as_future(std::move(cert_chain_f));
                f.ignore_ready_future();
                co_await close_stream(std::move(stream));
                continue;
            }
            else if (cert_chain_f.failed()) {
                auto eptr = cert_chain_f.get_exception();
                logger.warn(
                    "{} TLS handshake failed on connection with {}: {} Ignoring connection and retrying...",
                    peer.hostname, eptr);
                co_await close_stream(std::move(stream));
                continue;
            }
            auto peer_certificate_chain = cert_chain_f.get(); // must be .available() if we got here
            // ...
            break;
  //...
}

[avikivity]

The way it's done now is to set up a timer and trigger the abort_source or socket shutdowns from the timer.

Then co_await the computation you want, and immediately cancel the timer. Seems simpler, no?

    timer<> timeout_timer([&] {
            stream->shutdown_input();  // this should trigger an exception
            stream->shutdown_output(); // in the TLS handshake
    });
    timeout_timer.arm(5s);
    auto cert_chain = co_await seastar::tls::get_peer_certificate_chain(stream->socket), &abort_source);
    timeout_timer.cancel();

[niekbouman]

Aha, yup that's simpler indeed! (I wasn't aware of the existence of timeout_timer.)