Skip to content

enclave: set a write deadline and check deadline errors on vsock conns - #74

Merged
nickpell merged 1 commit into
mainfrom
nick/openauction-enclave-write-deadline
Sep 24, 2026
Merged

nickpell merged 1 commit into
mainfrom
nick/openauction-enclave-write-deadline

Conversation

@nickpell

@nickpell nickpell commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

Summary

  • handleConnection in enclave/server.go now sets a write deadline (30 s, starting once the request is handled) before encoding the response. Without it, a peer that stops reading holds its worker slot until the connection closes, and enough such peers exhaust the ENCLAVE_MAX_WORKERS pool.
  • A failure from SetReadDeadline or SetWriteDeadline is logged and the connection is closed, instead of being ignored and the connection served with no deadline.
  • Same change as enclave-server: set a write deadline and check deadline errors on vsock conns openarbiter#34; the two PRs are independent and can merge in either order.
  • enclave/server_test.go covers both paths: a peer that never reads the response releases the handler at the write deadline, and a failed SetReadDeadline closes the connection without reading.

Pre-merge checklist

  • Tests pass (mise run //:test, including the new TestHandleConnection_* tests)
  • Lint passes (mise run //:lint, mise run //:tidy)
  • Diff contains no unintended changes

Post-deploy/apply verification

  • EIF build for the merge commit succeeds and its PCR measurements commit lands on main (Build EIF run 36014574229; PCR commit 372ee59)
  • New enclave image rolled out and ping health checks pass (staging and production rolled out 2026-09-24; every enclave on the new image answering ping)

🤖 Generated with Claude Code

A peer that stops reading the response held its worker slot until the
connection closed. The response write now has a deadline, and a failure to
set either deadline closes the connection instead of serving it unbounded.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The deadline handling is correct, closes failed connections, and has focused regression coverage.

Review effort: Balanced
Findings: None

What changed in this PR

Adds connection deadline handling to prevent stalled vsock peers from exhausting enclave workers.

Changes:

  • Enforces and validates 30-second read/write deadlines.
  • Adds tests for stalled writes and read-deadline failures.
File Description
enclave/​server.go Adds deadline configuration and error handling.
enclave/​server_test.go Tests deadline-related connection behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@nickpell
nickpell merged commit 419a189 into main Sep 24, 2026
4 checks passed
@nickpell
nickpell deleted the nick/openauction-enclave-write-deadline branch September 24, 2026 14:36
nickpell added a commit to cloudx-io/openarbiter that referenced this pull request Sep 24, 2026
…ck conns (#34)

## Summary

- `handleConnection` in `cmd/enclave-server/main.go` now sets a write
deadline (30 s, starting once the request is handled) before encoding
the response. Without it, a peer that stops reading holds its worker
slot until the connection closes, and enough such peers exhaust the
`ENCLAVE_MAX_WORKERS` pool.
- A failure from `SetReadDeadline` or `SetWriteDeadline` is logged and
the connection is closed, instead of being ignored and the connection
served with no deadline.
- Same change as cloudx-io/openauction#74; the two PRs are independent
and can merge in either order.
- `cmd/enclave-server/main_test.go` covers both paths: a peer that never
reads the response releases the handler at the write deadline, and a
failed `SetReadDeadline` closes the connection without reading.

## Pre-merge checklist

- [x] Tests pass (`mise run //:test`, including the new
`TestHandleConnection_*` tests)
- [x] Lint passes (`mise run //:lint`, `mise run //:tidy`)
- [x] Diff contains no unintended changes

## Post-deploy/apply verification

- [ ] EIF build for the merge commit succeeds and its PCR measurements
commit lands on `main`
- [ ] New enclave image rolled out and ping health checks pass

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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