Skip to content

Remove unambiguous local shared-ownership taxes from runtime hot paths #640

Description

@leynos

Summary

Remove four behaviour-preserving shared-ownership operations that do not correspond to independent lifetimes.

This is the low-risk first implementation slice of Epic #635 and ADR 011 proposal #636. It should remain reviewable as a local refactor rather than waiting for the prepared-application or client-pool redesigns.

Changes

1. Borrow the connection actor cancellation token

ConnectionActor::next_event currently clones self.shutdown and passes the owned token to wait_shutdown, which calls cancelled_owned().

Refactor the select loop to borrow the token and await CancellationToken::cancelled() for the duration of one loop iteration. Extract disjoint field references before tokio::select! as needed to satisfy the borrow checker.

Do not change cancellation precedence or the existing biased branch order.

2. Send through the borrowed push sender

PushHandle::push_with_priority currently calls tx.clone().send(frame).await even though mpsc::Sender::send takes &self.

Call tx.send(frame).await directly. Preserve error mapping, rate-limit ordering, and queue backpressure.

3. Store the connection-local fragmenter by value

ConnectionActor is the sole owner of its configured fragmenter, but stores Option<Arc<Fragmenter>>.

Change this to Option<Fragmenter> and borrow it during frame fragmentation. Do not change the public Fragmenter concurrency contract or replace its atomic message-id source in this issue; a connection-local non-atomic allocator can be evaluated separately with evidence.

4. Store the middleware function directly in FnService

FromFn::transform clones F, places it in a fresh Arc, and creates one FnService that is never cloned.

Store F directly in FnService<S, F> and invoke it by reference. Preserve all current Send + Sync + 'static bounds and middleware behaviour.

Acceptance criteria

  • ConnectionActor::next_event performs no CancellationToken clone per event-loop iteration.
  • Cancellation remains first in the biased select order and all shutdown tests pass.
  • PushHandle::push_with_priority performs no sender clone immediately before send.
  • Push backpressure, rate limiting, queue closure, and loom tests pass unchanged.
  • ConnectionActor contains Option<Fragmenter>, not Option<Arc<Fragmenter>>.
  • Fragment message identifiers and fragmentation output remain unchanged.
  • FnService owns F directly and allocates no Arc<F> during transformation.
  • Middleware ordering and request/response tests pass.
  • The relevant Establish runtime ownership and task-churn baselines #639 benchmark scenarios are run before and after, with results recorded in the PR or benchmark notes.
  • No public API change is introduced.

Tests

Add or retain focused tests that make the borrowed-lifetime behaviour visible indirectly:

  • repeated actor events followed by cancellation terminate promptly;
  • cancellation already requested before run still exits immediately;
  • sender closure returns PushError::Closed without cloning requirements;
  • fragmenter state advances across multiple frames owned by one actor;
  • middleware functions with captured immutable state still execute correctly.

Non-goals

  • Redesigning CancellationToken use across independent tasks.
  • Removing legitimate shared state from PushHandle.
  • Introducing a connection-local non-atomic fragment ID allocator.
  • Changing the middleware trait or public from_fn API.

References

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions