feat(managed): add blocking managed owner #51

Merged
Lochnair merged 3 commits from blocking-managed-owner into main 2026-08-09 13:07:51 +00:00
Lochnair commented 2026-08-09 12:26:29 +00:00 (Migrated from github.com)

Summary

Adds a blocking ownership adapter for the unified Tokio managed client.

  • Runs one ManagedClientTask on one dedicated OS thread with a Tokio
    current-thread runtime.
  • Exposes cloned ManagedClientHandle values for status, events, stop, and
    dynamic target updates.
  • Keeps exactly one synchronous completion owner through
    BlockingManagedClient::join.
  • Returns the task's authoritative ManagedOutcome unchanged.
  • Graceful Drop requests stop and joins the worker instead of detaching it.
  • Reports worker panics separately from managed driver outcomes.
## Summary Adds a blocking ownership adapter for the unified Tokio managed client. - Runs one `ManagedClientTask` on one dedicated OS thread with a Tokio current-thread runtime. - Exposes cloned `ManagedClientHandle` values for status, events, stop, and dynamic target updates. - Keeps exactly one synchronous completion owner through `BlockingManagedClient::join`. - Returns the task's authoritative `ManagedOutcome` unchanged. - Graceful Drop requests stop and joins the worker instead of detaching it. - Reports worker panics separately from managed driver outcomes.
chatgpt-codex-connector[bot] (Migrated from github.com) reviewed 2026-08-09 12:33:22 +00:00
chatgpt-codex-connector[bot] (Migrated from github.com) left a comment

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 749306394c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

### 💡 Codex Review Here are some automated review suggestions for this pull request. **Reviewed commit:** `749306394c` <details> <summary>ℹ️ About Codex in GitHub</summary> <br/> [Your team has set up Codex to review pull requests in this repo](https://chatgpt.com/codex/cloud/settings/general). Reviews are triggered when you - Open a pull request for review - Mark a draft as ready - Comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". </details>
@@ -0,0 +1,156 @@
use std::{
chatgpt-codex-connector[bot] (Migrated from github.com) commented 2026-08-09 12:33:22 +00:00

P1 Badge Bound runtime shutdown after cancelled DNS lookups

When a hostname target is still resolving as the owner is stopped, resolve_remote_tokio has a tokio::net::lookup_host operation running in Tokio's blocking pool; cancelling the managed connect future does not cancel that underlying resolver call. After block_on returns, this closure implicitly drops the runtime, and Tokio's runtime destructor waits indefinitely for blocking work, so join() or drop(owner) can hang despite the documented bounded cleanup. Explicitly shut down the dedicated runtime with a timeout (or in the background) after capturing the task outcome.

Useful? React with 👍 / 👎.

**<sub><sub>![P1 Badge](https://img.shields.io/badge/P1-orange?style=flat)</sub></sub> Bound runtime shutdown after cancelled DNS lookups** When a hostname target is still resolving as the owner is stopped, `resolve_remote_tokio` has a `tokio::net::lookup_host` operation running in Tokio's blocking pool; cancelling the managed connect future does not cancel that underlying resolver call. After `block_on` returns, this closure implicitly drops the runtime, and Tokio's runtime destructor waits indefinitely for blocking work, so `join()` or `drop(owner)` can hang despite the documented bounded cleanup. Explicitly shut down the dedicated runtime with a timeout (or in the background) after capturing the task outcome. Useful? React with 👍 / 👎.
@@ -0,0 +111,4 @@
runtime.shutdown();
outcome
})
.map_err(|source| BlockingManagedStartError::ThreadSpawn { source })?;
chatgpt-codex-connector[bot] (Migrated from github.com) commented 2026-08-09 12:33:22 +00:00

P2 Badge Preserve the thread-spawn error inside async callers

When the OS rejects thread creation and start is called from a Tokio async task, the rejected closure—and therefore its captured runtime—is dropped on the caller thread. Tokio's normal runtime destructor panics when it attempts its blocking shutdown from an asynchronous context, so this path can panic instead of returning the documented BlockingManagedStartError::ThreadSpawn. Constructing the runtime inside the worker, or explicitly performing a nonblocking shutdown when spawning fails, preserves the recoverable error contract.

Useful? React with 👍 / 👎.

**<sub><sub>![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat)</sub></sub> Preserve the thread-spawn error inside async callers** When the OS rejects thread creation and `start` is called from a Tokio async task, the rejected closure—and therefore its captured `runtime`—is dropped on the caller thread. Tokio's normal runtime destructor panics when it attempts its blocking shutdown from an asynchronous context, so this path can panic instead of returning the documented `BlockingManagedStartError::ThreadSpawn`. Constructing the runtime inside the worker, or explicitly performing a nonblocking shutdown when spawning fails, preserves the recoverable error contract. Useful? React with 👍 / 👎.
@@ -0,0 +1,355 @@
use std::{
chatgpt-codex-connector[bot] (Migrated from github.com) commented 2026-08-09 12:33:22 +00:00

P2 Badge Avoid subscribing after the short test has started

This subscription is created only after the dedicated worker has started, while broadcast events are not replayed. If the caller is descheduled long enough for this 120 ms run to finish, subscribe() returns Closed; if only the early sends are missed, the later receive loop can instead time out waiting for an event that already occurred. This makes the lifecycle test scheduler-dependent, so it should wait on durable status or another synchronization signal that cannot be missed.

Useful? React with 👍 / 👎.

**<sub><sub>![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat)</sub></sub> Avoid subscribing after the short test has started** This subscription is created only after the dedicated worker has started, while broadcast events are not replayed. If the caller is descheduled long enough for this 120 ms run to finish, `subscribe()` returns `Closed`; if only the early sends are missed, the later receive loop can instead time out waiting for an event that already occurred. This makes the lifecycle test scheduler-dependent, so it should wait on durable status or another synchronization signal that cannot be missed. Useful? React with 👍 / 👎.
Sign in to join this conversation.