Skip to content

feat: add remote Deadpool executor - #429

Merged
cjrh merged 24 commits into
mainfrom
feat-ipc-submission
Jul 30, 2026
Merged

feat: add remote Deadpool executor#429
cjrh merged 24 commits into
mainfrom
feat-ipc-submission

Conversation

@cjrh

@cjrh cjrh commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Big picture

Add an experimental remote executor so independent processes can share one
server-owned Deadpool over Unix-domain or TCP sockets while retaining the
standard concurrent.futures.Executor programming model. The implementation
keeps application payloads opaque in the broker, makes distributed ownership
and cancellation outcomes explicit, and bounds network-facing queues, frames,
connections, and retained results.

The existing single-file module is migrated into a package so the remote
client/server implementation can be separated behind focused internal modules
without breaking current import deadpool users.

Changes

  • Preserve the existing local executor API through package-level re-exports and
    compatibility forwarding while moving the pool implementation to
    deadpool/_pool.py.
  • Add DeadpoolClient, DeadpoolServer, and RemoteFuture, including callable
    and registered-task submission, ordered mapping, health/status/statistics,
    request identity, fork protection, and explicit client/server lifecycle
    handling.
  • Add a private experimental wire protocol with safe JSON control headers,
    opaque chunked payloads, digest validation, bounded framing, typed failures,
    and ordered full-duplex transport handling.
  • Support protected Unix-domain sockets, explicit loopback TCP, and
    caller-configured mutual TLS with bounded authentication and connection
    admission.
  • Add broker-owned priority and principal-fair scheduling, queue and execution
    timeouts, ordinary/group/hard cancellation, terminal arbitration, and bounded
    result retention with acknowledgement cleanup.
  • Add serializer limits, worker-side opaque invocation decoding, remote
    traceback fallbacks, and distinct transport, compatibility, queue, process,
    cancellation, and result exceptions.
  • Document the remote API, pickle trust boundary, experimental wire status, and
    intentionally deferred protocol extensions.
  • Add package compatibility, protocol, transport, cancellation, timeout,
    security, retention, callback, fork, and end-to-end integration coverage.

Validation

  • nox -s test-3.14 — 103 passed.
  • Ruff checks for the new package and tests passed.
  • Wheel construction succeeded and includes all remote executor modules.

cjrh added 5 commits July 29, 2026 16:34
## Big picture

Add an experimental remote executor so independent processes can share one
server-owned Deadpool over Unix-domain or TCP sockets while retaining the
standard `concurrent.futures.Executor` programming model. The implementation
keeps application payloads opaque in the broker, makes distributed ownership
and cancellation outcomes explicit, and bounds network-facing queues, frames,
connections, and retained results.

The existing single-file module is migrated into a package so the remote
client/server implementation can be separated behind focused internal modules
without breaking current `import deadpool` users.

## Changes

- Preserve the existing local executor API through package-level re-exports and
  compatibility forwarding while moving the pool implementation to
  `deadpool/_pool.py`.
- Add `DeadpoolClient`, `DeadpoolServer`, and `RemoteFuture`, including callable
  and registered-task submission, ordered mapping, health/status/statistics,
  request identity, fork protection, and explicit client/server lifecycle
  handling.
- Add a private experimental wire protocol with safe JSON control headers,
  opaque chunked payloads, digest validation, bounded framing, typed failures,
  and ordered full-duplex transport handling.
- Support protected Unix-domain sockets, explicit loopback TCP, and
  caller-configured mutual TLS with bounded authentication and connection
  admission.
- Add broker-owned priority and principal-fair scheduling, queue and execution
  timeouts, ordinary/group/hard cancellation, terminal arbitration, and bounded
  result retention with acknowledgement cleanup.
- Add serializer limits, worker-side opaque invocation decoding, remote
  traceback fallbacks, and distinct transport, compatibility, queue, process,
  cancellation, and result exceptions.
- Document the remote API, pickle trust boundary, experimental wire status, and
  intentionally deferred protocol extensions.
- Add package compatibility, protocol, transport, cancellation, timeout,
  security, retention, callback, fork, and end-to-end integration coverage.

## Validation

- `nox -s test-3.14` — 103 passed.
- Ruff checks for the new package and tests passed.
- Wheel construction succeeded and includes all remote executor modules.
## Big picture

Strengthen the experimental remote executor with realistic behavioral coverage
across its public client, server, and Future interfaces. The expanded suite
exercises process, network, TLS, scheduling, cancellation, shutdown, and Unix
socket lifecycles rather than targeting individual implementation lines merely
to improve the percentage.

The new scenarios exposed several real races and lifecycle defects. Fix those
issues alongside the tests that reproduce them, and configure Coverage.py to
attribute work performed by multiprocessing, forked, and spawned children
correctly. Branch-aware coverage rises from roughly 79% to 88%, while statement
coverage rises above 90%.

## Changes

- Make submission registration atomic with transport failure and shutdown so a
  Future cannot be stranded after sender exit or concurrent client closure.
- Resolve control RPCs exactly once so a valid response cannot be overwritten by
  a subsequent disconnect, and ensure failed queueing releases RPC bookkeeping.
- Make all concurrent server startup callers observe the same readiness or
  startup failure, preserve failed startup state, and avoid request-table
  mutation while applying disconnect cancellation.
- Correct signed 64-bit protocol validation and reject inconsistent empty group
  identifiers before submission.
- Harden Unix listener cleanup with inode/device identity checks so failed bind
  and shutdown paths preserve visible replacement sockets and unrelated files.
- Configure coverage collection through `.coveragerc` for threads,
  multiprocessing, raw forks, subprocesses, and `_exit`, erase stale data before
  collection, and require a Coverage.py version that supports process patches.
- Add public end-to-end scenarios for spawned clients sharing one server pool,
  mutual TLS authentication and certificate rejection, chunked payloads,
  graceful draining, worker death and recovery, admission limits, scheduling
  fairness, disconnect policies, callbacks, and control/status operations.
- Add focused protocol, serializer, scheduler, configuration, and Future tests
  where low-level resource and state-machine invariants cannot be exercised
  reliably through the public API.
- Add test-only TLS credentials and document that they are fixtures with no
  production trust.
- Replace timing-sensitive cancellation assertions with explicit barriers,
  bounded polling, and robust child-process cleanup.

## Validation

- `nox -s test-3.14` — 172 passed.
- `nox -s testcov-3.14` — 172 passed; 88% branch-aware coverage.
- `nox -s test-3.10 -- tests/remote -q` — 105 passed.
- Ruff, Black, and `git diff --check` passed.
- Independent concurrency and test-quality review approved the final changes.
Comment thread deadpool/remote/server.py Outdated
Comment thread deadpool/remote/server.py Outdated
Comment thread deadpool/remote/config.py
Comment thread deadpool/remote/_future.py Outdated
Comment thread deadpool/remote/_worker.py
Comment thread deadpool/remote/server.py
Comment thread deadpool/remote/_future.py Outdated

@cjrh cjrh left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intended decision: Request Changes

Review scope

Reviewed PR #429 (feat: add remote Deadpool executor) at 9c62ae5d194069144809617570bfbbb926d60abd against main at 7412f7a64f8806dd6ebb6e68720529c2c7478ef5. I inspected the full 40-file diff plus the surrounding pool, client/server, protocol, transport, serializer, and test code. The untracked REMOTE_EXECUTOR_SPEC.md is not part of the PR and was excluded. There is no linked Shortcut story/issue and there were no existing PR conversation, review, or inline comments.

Blocking findings

I left line-level comments for each issue:

  1. Live-session removal leaks global admission capacitydeadpool/remote/server.py:1159. After the first request on a still-connected client is acknowledged, the session is removed from the server indexes. Later ACKs cannot find that session, so requests and worst-case result reservations leak permanently. This is a common sequential-use path and eventually produces RemoteQueueFull for valid clients.
  2. Startup can report failure or shutdown, then publish a live listenerdeadpool/remote/server.py:300. start() uses the handshake timeout for pool/listener initialization, while _initialize() continues and later writes RUNNING without checking whether timeout handling or concurrent shutdown already won.
  3. The client permits non-loopback plaintext TCPdeadpool/remote/config.py:37. TcpAddress("example.com", ..., insecure=True) is accepted, contrary to the documented loopback-only plaintext policy, exposing authentication and pickle payloads on the network.
  4. Cancelled RemoteFutures do not complete stdlib waitersdeadpool/remote/_future.py:151. done() becomes true, but concurrent.futures.wait() keeps the future in not_done and as_completed() times out because the base state never reaches CANCELLED_AND_NOTIFIED.
  5. A large task exception tears down the entire sessiondeadpool/remote/_worker.py:72. Traceback strings are unbounded even though protocol control strings are bounded; a 20 KiB exception message produced RemoteConnectionLost instead of a task failure and also fails unrelated in-flight work.
  6. Framing limits are neither negotiated nor rejecteddeadpool/remote/server.py:566. A valid 8 KiB-frame server and default client completed the handshake, then a 20 KiB submission inside the advertised invocation limit failed as SubmissionOutcomeUnknown.

Important follow-up

  • RemoteFuture.add_done_callback() reserves callback capacity synchronously (deadpool/remote/_future.py:74). With callback_queue_size=1, a second callback registration blocks until the future completes and can deadlock the thread needed to let the task finish. Bound dispatch rather than registration, or provide a nonblocking/inline fallback.
  • The primary README remote example starts a non-daemon server without shutting it down, then creates a second unstarted server and reuses executor after its context has closed (README.rst:785-805). Please replace it with one nested server/client context that demonstrates both modes and exits cleanly.
  • Registered mode still unpickles client-controlled arguments before invoking the registered function. The trust warning should say that the default PickleSerializer grants worker-account code execution in both callable and registered modes; operation authorization is not a sandbox.

Validation evidence

  • nox -s test-3.14passed, 172 tests; one existing os.fork() deprecation warning.
  • nox -s testcov-3.14passed, 172 tests; local report: 85% total branch-aware coverage.
  • ruff check deadpool/remote deadpool/__init__.py deadpool/_pool.py tests/remote tests/test_package_api.py noxfile.pypassed.
  • black --check deadpool tests noxfile.pypassed, 32 files unchanged.
  • git diff --check origin/main...HEADpassed.
  • python -m pip wheel . --no-deps --wheel-dir /tmp/deadpool-review-wheelpassed; the wheel contains all deadpool.remote modules.
  • nox -s lintcould not run with the installed Ruff because this nox session invokes the removed ruff . CLI form; the focused ruff check command above passed. This is a local tool-version mismatch, not evidence of a changed-file lint defect.
  • python -m build --wheel --sdist --outdir /tmp/deadpool-review-distcould not run because the active environment lacks the build module; pip wheel provided alternate package-build evidence.
  • Focused inline Python probes reproduced all six blocking behaviors and the callback-registration deadlock described above.
  • GitHub's Python 3.10, 3.11, 3.12, 3.13, 3.14, and 3.14t jobs, lint, dependency review/scan, Semgrep, CodeQL, and Trivy are green. Coveralls is failing: “Coverage decreased (-5.6%) to 88.172%.”

The broad automated suite is strong, but it does not exercise these lifecycle, stdlib interoperability, asymmetric-limit, and bounded-error paths. They are material correctness/security risks in the new remote executor and should be fixed with focused regressions before merge.

@cjrh cjrh left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intended decision: Request Changes

I reviewed PR #429 (main at 7412f7afeat-ipc-submission at 0e9c738), including the complete 40-file diff, remote protocol/client/server/package context, tests, prior threads, and CI. There is no linked Shortcut story or issue. The seven earlier inline threads each have a follow-up reply and their original issues appear addressed.

Blocking findings

  1. deadpool/remote/server.py:835 — local backlog backpressure disables remote queue deadlines.
    _dispatch_loop() calls the local pool's blocking private _submit_future() from the same thread that runs _expire_queued_locked(). Deadpool._submit_future() performs a blocking PriorityQueue.put(). Once the local pool's max_backlog is full, this thread cannot revisit expiration, so an accepted remote request can remain ACCEPTED_QUEUED past deadpool_queue_timeout until unrelated local work completes.

    I reproduced this with Deadpool(max_workers=1, max_backlog=1): after a running task and one local-backlog task, a third request with deadpool_queue_timeout=0.05 was still ACCEPTED_QUEUED after 0.25 seconds and became RemoteQueueTimeout only after releasing the unrelated running task. Use nonblocking/timed local admission with requeue/defer semantics, or separate deadline expiration from the blocking local-pool admission path. Add a regression that requires timeout completion before the saturated local backlog is released.

  2. deadpool/remote/config.py:11, :43-44, :98-99 — plaintext TCP accepts an unresolved hostname as “loopback.”
    The allowlist accepts the string localhost, but socket.create_connection() and socket.bind() resolve it later. A modified hosts/DNS resolver can map that name to a LAN address, making an explicitly insecure listener/client non-loopback despite the advertised restriction. With pickle invocation this can expose unencrypted, unauthenticated worker-account code execution. Require numeric loopback addresses for insecure TCP, or resolve every selected sockaddr and require ipaddress.ip_address(address).is_loopback; add a hostile-resolution regression.

  3. deadpool/remote/_protocol.py:374 — incomplete chunk buffering has no aggregate byte limit.
    max_incomplete_messages limits only IDs; every payload is retained in state.parts until the final chunk. An authenticated peer can open 16 logical 64-MiB default messages and withhold one final 1-MiB chunk from each, retaining about 0.984 GiB on one connection. At max_connections_global=256, that is roughly 252 GiB. This happens during framing, before operation authorization. Track and cap aggregate incomplete bytes per reader/connection (with a RemoteLimits setting), releasing the count on completion/error; add an interleaved-partial-message limit test.

  4. deadpool/remote/_future.py:70-76 — callbacks registered after completion do not follow concurrent.futures.Future semantics.
    A completed Future queues the callback through the dispatcher rather than invoking it before add_done_callback() returns. If the dispatcher is waiting on callback capacity, it can be deferred indefinitely. concurrent.futures.Future requires callbacks added after terminal completion to be called immediately. Invoke terminal callbacks inline with the existing exception logging; unfinished-Future registration can remain nonblocking. Add a regression with a stalled dispatcher that asserts a late callback has run when add_done_callback() returns.

  5. README.rst:823-825 — the pickle trust-boundary warning incorrectly scopes RCE to callable mode.
    Registered task arguments are also processed by PickleSerializer.loads() in deadpool/remote/_worker.py:35, before the registered function runs. A registry restricts handler names, not hostile pickle execution. State that the default serializer deserializes callable and registered-task payloads (and result/exception payloads), and that every peer permitted to submit either mode must be mutually trusted.

Validation

Passed locally:

  • python3 -m pytest -q tests/remote tests/test_package_api.py — 120 passed; one existing os.fork() deprecation warning.
  • nox -s testcov-3.14 — 185 passed; one existing warning; 87% branch-aware total coverage.
  • black --check deadpool tests/remote tests/test_package_api.py && ruff check deadpool tests/remote tests/test_package_api.py && git diff --check 7412f7a64f8806dd6ebb6e68720529c2c7478ef5..HEAD — passed (Black emitted only its Python-3.15 parsing warning).
  • uv build --wheel --out-dir /tmp/pr429-review-wheel — passed; wheel includes deadpool/__init__.py and all 11 remote modules.

GitHub's Python 3.10–3.14t test matrix, lint, CodeQL, Semgrep, Trivy, and dependency checks pass. coverage/coveralls currently fails because coverage decreased 6.0% to 87.78%; restore the required check before merging.

@wallies

wallies commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Review — remote Deadpool executor

Solid PR overall. CI is green, the _staged / outcome-reservation accounting and
the _begin_dispatch_commit_running/_abort_dispatch lock handoff all trace
out correctly, and I stress-tested the broker under concurrent submit/cancel/
timeout without finding any accounting leak or deadlock. No blockers below, but a
few validated issues worth addressing.

1. RESULT that fails client-side deserialization leaks the future and the server reservation (bug)

client.py _complete (RESULT branch): serializer.loads(message.payload) is
unguarded. If a result pickles on the worker but can't be unpickled on the client
(missing class/module), the exception is set on the future but acknowledge
stays False, so the finally skips _forget, _terminal_received.discard, and
the RESULT_ACK.

Reproduced with a client serializer whose loads raises for one result:

  • the request stays in client._futures forever (memory leak)
  • it stays in client._terminal_received forever (unbounded set growth)
  • the server never gets the ACK → retained_outcomes=1, reserved_bytes=33554432
    pinned until disconnect

The TASK_ERROR branch already wraps its loads in try/except — the RESULT
branch should acknowledge + forget on decode failure the same way. The
outbound-queue-full path in _complete's finally (_enqueue_control raising
RemoteConnectionLost) is the same leak class via a second trigger.

2. Full invocation payload is sent over the worker pipe while the global lock is held (liveness)

_begin_dispatch acquires self._condition and holds it across
worker.submit_job(...). The job tuple still embeds record.payload (it's cleared
to b"" only after _submit_future returns), so Connection.send writes the
entire serialized invocation — up to max_invocation_bytes (default 32 MiB) —
through the pipe under the lock. When the payload exceeds the OS pipe buffer the
send blocks mid-write, stalling every other submit/cancel/complete/dispatch behind
it (head-of-line blocking). If the target worker is genuinely stuck the send never
returns and the lock is never released → whole-broker deadlock.

Suggest committing RUNNING state under the lock but performing submit_job after
releasing it (accepting the small wasted-work-on-cancel-race the current design
avoids).

3. _send_exact has no total-time bound (write-side slow-loris)

_protocol.py _send_exact resets deadline = monotonic() + timeout after every
partial send(), so partial_frame_timeout bounds only the gap between progress,
not the whole write. A peer draining ~1 byte per interval keeps a writer thread
pinned indefinitely. Reproduced: a 200-byte frame with a 0.05 s timeout took 1.03 s
and never raised. _recv_exact sets its deadline once and is correct — the send
side should match it.

4. O(n)-under-lock scalability cluster (server broker)

All bounded by the configured caps, so scaling ceilings rather than bugs, but they
grow with backlog and all run under the single global lock:

  • _expire_queued_locked scans all requests (including retained terminal
    outcomes awaiting ACK) every 50 ms, even when idle.
  • _submit does an O(n) principal scan on every submission — a per-principal
    counter (like the existing reserved counters) would make it O(1).
  • FairScheduler.remove is O(n) + heapify; group-cancel / disconnect of a large
    queued backlog is effectively quadratic.

5. No idle timeout / partial-message eviction

After the handshake the socket is settimeout(None) and MessageReader._messages
has only a count bound (max_incomplete_messages), no time-based eviction. A peer
can hold ~1 GiB of partial reassembly buffers per connection open indefinitely,
with no reclamation.

Minor

  • RemoteFuture.cancel() blocks on a network RPC and can raise, deviating from the
    non-blocking stdlib Future.cancel() contract (can stall Executor.map cleanup
    and shutdown(cancel_futures=True) if the server is slow).
  • Deeply nested control JSON raises a raw RecursionError from _json_loads (the
    depth > 12 guard runs after json.loads); it's caught by the broad connection
    handlers so it's a clean disconnect, but wrapping it in RemoteProtocolError
    would be more consistent.
  • The invocation payload is SHA-256'd twice per submit (control digest + frame
    digest) — could pass the precomputed digest into send_message.

cjrh added 2 commits July 30, 2026 02:38
## Big picture

Keep the remote executor's public timing, scheduling, and trust guarantees intact under real backpressure and completion races. Remote queue deadlines must remain enforceable when the local pool is saturated, `RemoteFuture` must retain the standard `Future` callback contract, and the default pickle transport must clearly communicate that trust is bidirectional in every submission mode.

## Detailed changes

- make broker-to-pool admission nonblocking without changing ordinary local `Deadpool.submit()` backpressure
- requeue rejected admissions without losing strict priority, principal fairness, or FIFO ordering within a scheduling stream
- arbitrate queue deadlines immediately before worker dispatch so expired work cannot start in an admission race
- invoke callbacks registered after terminal completion synchronously while retaining isolated asynchronous dispatch for callbacks registered earlier
- centralize callback exception handling and the default `PickleSerializer` trust warning
- document that callable and registered-task invocations, results, and exceptions require mutually trusted clients and servers
- add regressions for saturated-backlog queue timeouts, scheduler requeue ordering, and late callback timing and logging
@cjrh

cjrh commented Jul 30, 2026

Copy link
Copy Markdown
Owner Author

@wallies Re item 1 in your review: fixed in dcefa4f.

A terminal result is now acknowledged and removed from the client tables even when client-side deserialization fails. If the mandatory RESULT_ACK cannot be queued, the client actively tears down the socket so server-side session cleanup releases the retained outcome reservation; local future and terminal-receipt entries are still removed.

Verified:

  • focused decode-failure and ACK-enqueue-failure regressions — 2 passed
  • python3 -m pytest -q tests/remote/test_client_api.py tests/remote/test_integration.py — 37 passed
  • ruff check deadpool/remote/client.py tests/remote/test_client_api.py — passed
  • git diff --check — passed

@cjrh

cjrh commented Jul 30, 2026

Copy link
Copy Markdown
Owner Author

@wallies Re item 2 in your review: fixed in 3bfa528.

The broker now atomically claims the queue/worker boundary and publishes the selected PID under its condition, then releases the condition before WorkerProcess.submit_job() writes the invocation to the pipe. This preserves queue-timeout/cancellation arbitration, lets hard cancellation terminate a blocked writer, and preserves broken-worker retries without double-counting the running request.

Verified:

  • focused blocked-pipe/hard-cancel and broken-pipe-retry regressions — 2 passed
  • python3 -m pytest -q tests/remote/test_scheduling_resilience.py tests/remote/test_cancellation.py tests/remote/test_integration.py — 40 passed
  • ruff check deadpool/remote/server.py tests/remote/test_scheduling_resilience.py — passed
  • git diff --check — passed
  • independent Python concurrency review — no blockers

@cjrh

cjrh commented Jul 30, 2026

Copy link
Copy Markdown
Owner Author

@wallies Re item 3 in your review: fixed in 0703344.

_send_exact() now uses one monotonic deadline for the complete write instead of refreshing it after each partial send, matching the receive-side total-time bound. The regression simulates continuous one-byte progress and verifies the write still times out before completion.

Verified:

  • python3 -m pytest -q tests/remote/test_protocol.py — 25 passed
  • ruff check deadpool/remote/_protocol.py tests/remote/test_protocol.py — passed
  • git diff --check — passed

@cjrh

cjrh commented Jul 30, 2026

Copy link
Copy Markdown
Owner Author

@wallies Re item 4 in your review: no change in this PR.

I confirmed the three costs, but they are bounded scaling ceilings rather than correctness failures: expiry and per-principal admission scan at most max_pending_global (default 4096), while group cancellation is additionally bounded by max_pending_per_session (default 1024). Replacing them would add a deadline index, principal lifecycle counters, and/or scheduler item indexes whose consistency would have to be maintained across acceptance, local-backlog requeue, ACK removal, disconnect, cancellation, and shutdown.

Without latency evidence at the configured ceilings, that added state and race surface is not justified for this experimental implementation. I’m deferring these optimizations to benchmark-driven follow-up work rather than expanding this correctness/security feedback pass.

@cjrh

cjrh commented Jul 30, 2026

Copy link
Copy Markdown
Owner Author

@wallies Re item 5 in your review: the material memory-amplification issue is fixed in 3792a4e.

MessageReader now caps aggregate buffered incomplete payload bytes per connection at max_message_bytes, in addition to the existing message-count and per-message bounds. With defaults, one connection can therefore retain at most 64 MiB of incomplete payload rather than 16 × 64 MiB (about 1 GiB). Completed messages release the aggregate charge.

I did not add a general post-handshake idle timeout: that would require keepalive/liveness semantics for legitimate long-running tasks and is a separate lifecycle feature. The aggregate cap provides the needed finite memory bound without introducing that policy or new configuration.

Verified:

  • python3 -m pytest -q tests/remote/test_protocol.py — 26 passed
  • ruff check deadpool/remote/_protocol.py tests/remote/test_protocol.py — passed
  • git diff --check — passed

@cjrh

cjrh commented Jul 30, 2026

Copy link
Copy Markdown
Owner Author

@wallies Re the blocking RemoteFuture.cancel() minor in your review: documented in 3f6d8b3; I did not change the semantics.

Once work is submitted, a truthful bool result requires the broker's authoritative cancellation decision. Making this nonblocking would require optimistic local cancellation (which can lie about remote execution), always returning False, or a separate asynchronous cancellation API. Each weakens the PR's explicit distributed-outcome model or expands its public API.

The README and method contract now state that submitted cancellation can block up to control_timeout or raise RemoteCancellationOutcomeUnknown; locally pending work still cancels immediately.

Verified:

  • python3 -m pytest -q tests/remote/test_cancellation.py — 9 passed
  • ruff check deadpool/remote/_future.py — passed
  • git diff --check — passed

@cjrh

cjrh commented Jul 30, 2026

Copy link
Copy Markdown
Owner Author

@wallies Re the recursive control-JSON minor in your review: fixed in 7d475f0.

_json_loads() now normalizes decoder RecursionError into RemoteProtocolError, matching its other malformed-JSON failures. Coverage verifies both the explicit protocol nesting guard and decoder-level recursion failure.

Verified:

  • python3 -m pytest -q tests/remote/test_protocol.py — 28 passed
  • ruff check deadpool/remote/_protocol.py tests/remote/test_protocol.py — passed
  • git diff --check — passed

@cjrh

cjrh commented Jul 30, 2026

Copy link
Copy Markdown
Owner Author

@wallies Re the duplicate invocation SHA-256 minor in your review: no change in this PR.

The two digests protect different layers: the submission digest is broker request-identity metadata, while send_message() independently binds the bytes it actually frames. Passing a caller-supplied digest into the framing layer would widen that internal interface and require every call path to preserve a new payload/digest invariant. Without profiling evidence that hashing is material relative to serialization and socket/pipe transfer, the coupling and stale-digest risk outweigh this optional optimization.

@cjrh

cjrh commented Jul 30, 2026

Copy link
Copy Markdown
Owner Author

Follow-up on the second owner review’s insecure-localhost finding: fixed in 561809e.

Rechecking the full feedback inventory showed this item was not part of the earlier consolidated 475696a change. Insecure TCP now accepts only the numeric 127.0.0.1 address for both clients and listeners; localhost is rejected before any later resolver choice can redirect plaintext traffic off-host. TLS configurations can continue using hostnames normally.

Verified:

  • python3 -m pytest -q tests/remote/test_tcp.py — 11 passed
  • ruff check deadpool/remote/config.py tests/remote/test_tcp.py — passed
  • git diff --check — passed

@cjrh
cjrh merged commit de7d179 into main Jul 30, 2026
14 of 15 checks passed
@cjrh
cjrh deleted the feat-ipc-submission branch July 30, 2026 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants