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

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    concurrencyConcurrency, parallelism, and synchronization work, including races and deadlocks.enhancementNew feature or requestlowBacklog work, hygiene, or technical debt planned opportunistically, without a strict deadline.performancerefactorBehaviour-preserving restructuring that improves code health.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions