Skip to content

Missing async deadlines and util::spawn context errors #8235

Description

@sublimator

Checked at develop 551a19b10d:

  1. HTTPClient: the deadline wait is inside a catch block, so successful setup
    leaves requests without a timeout. Fallback error state and TLS timeout cleanup
    also need fixing.
  2. ConnectAttempt: the timer is canceled before reading the upgrade response;
    a silent TLS peer can retain the attempt indefinitely.
  3. util::spawn: plain context/executor arguments fail to compile in the tested
    Boost 1.91 configuration. Passing them directly to make_strand fixes it.

Timeout regressions failed before and passed after the fixes in a downstream
Boost 1.91/GCC 11 build. The spawn probe used the upstream header directly.
Fixes and reproductions are below.

1. HTTPClient: arming, fallback state, and TLS cleanup

httpsNext()
registers async_wait only when expires_after throws. This affects callers
such as rpc_call::fromNetwork, which requests a 30-second timeout.
Before #5570, the wait was guarded by successful expiry setup.
Restore that ordering, keeping the existing resolver/error-completion code below:

shutdown_.clear();
try
{
    deadline_.expires_after(timeout_);
}
catch (boost::system::system_error const& e)
{
    shutdown_ = e.code();
    JLOG(j_.trace()) << "expires_after: " << shutdown_.message();
}
if (!shutdown_)
{
    deadline_.async_wait(
        [self = shared_from_this()](boost::system::error_code const& ec) {
            self->handleDeadline(ec);
        });
}

The explicit reset also restores the old error-code overload's behavior:
without it, a failed host leaves shutdown_ set and prevents fallback.

On expiry, graceful TLS shutdown can itself wait for a silent peer. Replace
that sequence in handleDeadline() with a transport close:

shutdown_ = boost::asio::error::timed_out;
resolver_.cancel();
boost::system::error_code ec;
socket_.lowestLayer().close(ec);
if (ec)
    JLOG(j_.trace()) << "Deadline close error: " << ec.message();

The pending operation's handler delivers completion. Preserve the timeout at
handleHeader() entry rather than parsing an incomplete header:

if (shutdown_)
{
    invokeComplete(shutdown_);
    return;
}

Reproduce: use local listeners, an initialized HTTPClient SSL context, a
one-second request timeout, and ioContext.run_for(std::chrono::seconds{4}).
For self-signed TLS fixtures, disable certificate verification in the test client.

  • Accept/read plain HTTP, then withhold the response.
  • Accept TCP but stall the TLS handshake.
  • Complete TLS/read the request, then withhold the response and TLS shutdown reply.
  • For fallback, call the deque-of-sites get overload with 127.0.0.2 then
    127.0.0.1 at the same port: reserve the first without listening; serve a
    complete HTTP 200 response on the second.

Check exactly one callback and a drained context before teardown: timeout
for the stalled cases, success/status/body for fallback. The plain case failed
before restoring the wait; the remaining cases still failed until the state and
transport fixes. Fixed TLS cases report timed_out.

2. ConnectAttempt: missing upgrade-response deadline

onWrite()
cancels the write timer, then starts an untimed response read. This
predates #5570.
Rearm after the existing error/socket checks:

+    setTimer();
     boost::beast::http::async_read(
         stream_,
         readBuf_,
         response_,

Reproduce: a loopback TLS peer completes the handshake, reads the upgrade
request, and keeps the socket open without replying. Bound the test at 20 seconds
for the existing 15-second timer. Without rearming, the attempt remains pending;
with it, the client closes and releases the attempt/slot. Check remote EOF/reset
and weak-pointer expiry before destroying the fixture. A stalled TLS handshake
is a control for the already-working handshake deadline; run both concurrently.

3. util::spawn: context/executor instantiation

The non-strand branch
was probed with Apple clang 21.0.0, arm64 macOS, C++23, Boost 1.91.0, and
BOOST_ASIO_USE_TS_EXECUTOR_AS_DEFAULT:

Lvalue argument Result
io_context Fails: association queries a const object, but get_executor() is non-const
io_context::executor_type Fails: association falls back to inline_executor, which lacks context(), on_work_started(), and on_work_finished()
strand<io_context::executor_type> Compiles and runs

Keep the existing strand branch; replace the lookup in the other branch:

-            boost::asio::make_strand(boost::asio::get_associated_executor(std::forward<Ctx>(ctx))),
+            boost::asio::make_strand(std::forward<Ctx>(ctx)),

Reproduce: compile separately with PROBE_CASE=0, 1, and 2, using the
project's include/link settings and the configuration above:

#include <xrpl/server/detail/Spawn.h>
#include <boost/asio/io_context.hpp>

#ifndef PROBE_CASE
#define PROBE_CASE 0
#endif

int main()
{
    boost::asio::io_context ioc;
#if PROBE_CASE == 0
    auto& ctx = ioc;
#elif PROBE_CASE == 1
    auto ctx = ioc.get_executor();
#else
    auto ctx = boost::asio::make_strand(ioc);
#endif
    xrpl::util::spawn(ctx, [](boost::asio::yield_context) {});
    ioc.run();
}

The instrumented control called boost::asio::spawn with make_strand(ctx)
and xrpl::util::impl::kPropagateExceptions directly. All three forms compiled
and ran. It also verified that the callback waits for ioc.run(), executes on
its calling thread, and propagates a specific thrown exception from ioc.run().
These results establish the tested configuration, not every possible context or
Boost version.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions