Skip to content

Replace spawn-on-demand pool servicing with a persistent scheduler actor #647

Description

@leynos

Summary

Replace the pool scheduler's Arc<Mutex<SchedulerState>> plus atomic service guard and transient tokio::spawn calls with one long-lived scheduler task that owns the fairness and waiter state.

This is the final structural implementation slice for ADR 013 proposal #638. It follows the stable slot graph in #645 and should consume the scoped permit-selection API from #646.

Current behaviour

The current scheduler:

  • shares SchedulerState through a synchronous mutex;
  • uses AtomicBool::is_servicing as a distributed task-lifecycle guard;
  • calls kick from acquire and lease-drop paths;
  • spawns service_waiters when the guard transitions from idle;
  • may spawn a task that immediately discovers no waiter and exits;
  • divides shutdown, waiter selection, restart recovery, and capacity notification among several caller contexts.

This topology makes one conceptual state machine look like shared mutable data. It also complicates #550's bounded-waiter admission and leaves adversarial interleavings from #593/#535 difficult to test directly.

Proposed architecture

Create one scheduler task during pool construction. It owns SchedulerState directly and receives commands through a Tokio channel.

A representative protocol is:

enum SchedulerCommand<S, P, C> {
    Acquire {
        handle_id: u64,
        reply: oneshot::Sender<Result<PooledClientLease<S, P, C>, ClientError>>,
    },
    DeregisterHandle {
        handle_id: u64,
    },
    CapacityAvailable,
    Shutdown {
        reply: oneshot::Sender<()>,
    },
}

The exact types may differ, but the protocol and ownership must remain explicit.

Ownership split

Keep scheduler control and physical slot storage separate:

  • Arc<PoolCore> from Introduce PoolCore and index-based pooled leases #645 owns fixed slots and shared physical resources;
  • a cloneable SchedulerHandle owns only the command sender and synchronous handle-ID allocation if retained;
  • SchedulerTask owns the receiver, SchedulerState, fairness rotation, waiter counts, and shutdown state;
  • the task may own Arc<PoolCore>, but PoolCore must not own the command sender in a way that creates a strong cycle.

Dropping the final public scheduler sender must eventually close the mailbox, terminate the task, and release the task's pool-core reference.

Responsibilities of the actor

Admission

Fairness

  • preserve FIFO arrival order for live admitted waiters;
  • preserve round-robin turns among logical handles;
  • handle deregistration idempotently;
  • never grant a lease to a cancelled or deregistered waiter.

Capacity

Shutdown

  • reject later acquires;
  • resolve every queued waiter with ClientError::disconnected or the chosen typed shutdown error;
  • cancel or drop any pending permit-selection future safely;
  • acknowledge explicit close() only after scheduler state has drained and the actor can terminate;
  • terminate on mailbox closure even when callers omit explicit close.

Public API behaviour

Preserve existing WireframeClientPool, PoolHandle, and PooledClientLease APIs where practical.

WireframeClientPool::handle() may remain synchronous by allocating a logical ID outside the actor. The implementation must prove command ordering for acquire and deregistration from one logical handle. A registration acknowledgement is acceptable if the public API change is justified, but do not quietly assume cross-sender global ordering that Tokio does not promise.

WireframeClientPool::close(self) becomes an awaitable scheduler shutdown rather than shutdown(); yield_now(); drop(self).

Drop behaviour

PooledClientLease::drop cannot await. It should:

  • release its OwnedSemaphorePermit by ordinary drop;
  • send or coalesce a non-blocking capacity hint to the actor;
  • tolerate an already-closed scheduler mailbox during shutdown;
  • avoid cloning a large ownership graph;
  • never spawn a task.

If permit release can be observed without a per-drop command, for example through a shared Notify polled by the actor, that is acceptable if it preserves the single-owner scheduler state and has no lost-wakeup race.

Integration with existing issues

Acceptance criteria

  • Exactly one scheduler task is created per client pool.
  • Repeated acquire/drop operations create no additional scheduler tasks.
  • SchedulerState is owned by the actor and is not behind Arc<Mutex<_>>.
  • The is_servicing atomic/spawn-restart protocol is removed.
  • Acquire, deregister, capacity, and shutdown transitions flow through an explicit command or wake protocol.
  • FIFO and round-robin semantics match the existing public contract.
  • Waiter admission is bounded according to [security][low] Client pool admits unbounded blocked waiters under backpressure #550.
  • Cancelled acquire futures are pruned and receive no lease.
  • Blocked waiters resolve promptly during shutdown.
  • close() waits for scheduler shutdown rather than yielding speculatively.
  • Dropping all public pool/control handles without close() terminates the actor and releases PoolCore.
  • Lease drop never calls tokio::spawn.
  • No strong-reference cycle retains the pool.
  • [security][medium] Cancelled pooled client operations can return dirty sockets as reusable #548's dirty-socket invariant is preserved.
  • Establish runtime ownership and task-churn baselines #639 benchmarks record uncontended/contended latency, allocations, and task-count changes.

Tests

Add deterministic state-machine and runtime tests covering:

  • immediate uncontended grant;
  • FIFO ordering under saturation;
  • round-robin ordering across several logical handles;
  • cancelled waiter before and during grant;
  • handle deregistration with queued waiters;
  • bounded waiter rejection;
  • capacity release racing with acquire and shutdown;
  • explicit close with blocked waiters;
  • implicit shutdown when all senders drop;
  • actor task count remaining one under a long acquire/drop loop;
  • no PoolCore retention after final handle/lease drop.

Use loom or a small pure scheduler-state model for event-order races that cannot be forced reliably with wall-clock Tokio tests.

Non-goals

  • Moving socket request/response I/O into the scheduler actor.
  • Dynamically resizing the slot collection.
  • Replacing Tokio channels or semaphores.
  • Weakening cancellation safety to simplify capacity accounting.

Dependencies

References

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions