Repository navigation
Conversation
Provide bounded borrowed-stream and subprocess transport, explicit STDIO helper selection, idle HTTP disconnect signals and remembered pre-registration cancellation. Preserve HTTP/1 pipelining and upgrade handoff with finite lookahead, and document independent SDK ownership and connection limits. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Jerome Louvel <374450+jlouvel@users.noreply.github.com>
Separate cleanup phases, preserve interruption causes, retain duration overflow validation, and avoid catching fatal errors. Use supported Javadoc markup and deterministic child blocking. Revalidated the 134-case live suite and strict Javadoc generation without relaxing safeguards. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Jerome Louvel <374450+jlouvel@users.noreply.github.com>
Reproduce the EOF-before-terminal-publication gap with a held exchange lock, then publish completion before waking readers. Extend lifecycle coverage with immediate closes on empty 401/403 responses and the existing authorization cases. The expanded live regression selector passes 145 cases together with strict Javadoc generation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Jerome Louvel <374450+jlouvel@users.noreply.github.com>
Collaborator
Author
|
Follow-ups aff5257 and afe0801 address strict Javadoc/static-analysis findings without changing safeguards, and close a deterministically reproduced client EOF-before-terminal-state publication gap found during CI investigation. The expanded live selector now passes 145 tests (zero failures/errors/skips), including 128 immediate empty-error response/client-close exchanges, all eight authorization cases and strict Javadoc generation. The held-lock regression was confirmed failing before the ordering fix. The latest head is undergoing CI; earlier failed runs are retained as evidence rather than treated as green. |
|
Collaborator
Author
|
@thboileau Could you review this PR, part of a stack of three preparation for foundation for support of gRPC, MCP and Smithy on top of Restlet 3 / Netty 4.2 reusing a maximum of the official SDKs/libraries? |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Pre-cancelled requests can leak connections, valid half-closed connections can lose responses, and protocol upgrades, disconnect signaling, and subprocess shutdown have unresolved failures.
10 open issues
What changed in this PR
Adds R1 protocol-neutral transport and request-lifecycle foundations on top of R0, preparing core integration points for independent MCP, gRPC and Smithy extensions without delivering production adapters.
Changes:
- Adds bounded byte-stream transport with explicit borrowed-stream and owned-subprocess lifecycle handling.
- Adds disconnect notifications, remembered cancellation and bounded HTTP/1.1 pipeline lookahead.
- Documents ownership and integration limits, with transport, lifecycle and module-boundary regressions.
| File | Description |
|---|---|
| readme.md | Documents R1 contracts, limits and validation. |
| org.restlet/src/test/java/org/restlet/util/StreamTransportTestCase.java | Tests byte limits, ownership and subprocess cleanup. |
| org.restlet/src/test/java/org/restlet/RequestLifecycleTestCase.java | Tests disconnect signals and cancellation registration races. |
| org.restlet/src/test/java/org/restlet/engine/netty/Http1LookaheadTestCase.java | Tests lookahead limits, buffer release and completion ordering. |
| org.restlet/src/test/java/org/restlet/engine/connector/StdioHelperSelectionTestCase.java | Tests explicit and unique STDIO helper selection. |
| org.restlet/src/test/java/org/restlet/engine/connector/RpcSdkBoundaryTestCase.java | Verifies public transport access and independent SDK module boundaries. |
| org.restlet/src/test/java/org/restlet/engine/connector/HttpDisconnectTestCase.java | Tests disconnects, cancellation, pipelining and empty error responses. |
| org.restlet/src/main/java/org/restlet/util/WrapperRequest.java | Delegates disconnect lifecycle operations. |
| org.restlet/src/main/java/org/restlet/util/StreamTransport.java | Exposes the supported protocol-neutral byte API. |
| org.restlet/src/main/java/org/restlet/Request.java | Adds first-cause disconnect signaling and remembered cancellation. |
| org.restlet/src/main/java/org/restlet/engine/stdio/StreamTransportHelper.java | Implements bounded I/O and stream/process lifecycle management. |
| org.restlet/src/main/java/org/restlet/engine/netty/NettyServerHandler.java | Adds bounded ordered lookahead and disconnect observation. |
| org.restlet/src/main/java/org/restlet/engine/netty/NettyServerCall.java | Connects server request lifecycle with response completion. |
| org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java | Reports failures and publishes completion before body EOF. |
| org.restlet/src/main/java/org/restlet/engine/Engine.java | Enforces unambiguous STDIO helper selection. |
| changes.md | Records transport additions and lifecycle behavior changes. |
Review effort: Balanced
Comment on lines
+349
to
+350
| if (action != null && abortRequested.get()) { | ||
| action.getAsBoolean(); |
Comment on lines
+840
to
+842
| org.restlet.Request request = currentRequest; | ||
| if (request != null) { | ||
| request.notifyDisconnect(cause); |
Comment on lines
+151
to
+153
| if (pending.isEmpty()) { | ||
| ctx.read(); | ||
| } |
Comment on lines
+253
to
+254
| if (kind == StreamKind.HTTP1 && readAfterBody && pending.isEmpty()) { | ||
| ctx.read(); |
Comment on lines
+253
to
+254
| if (kind == StreamKind.HTTP1 && readAfterBody && pending.isEmpty()) { | ||
| ctx.read(); |
Comment on lines
+255
to
+256
| synchronized void onDisconnect(Throwable cause) { | ||
| if (!completed && disconnectCause == null) { |
|
|
||
| public int read(byte[] buffer, int offset, int length) throws IOException { | ||
| Objects.checkFromIndexSize(offset, length, buffer.length); | ||
| lock(readLock); |
Comment on lines
+180
to
+183
| if (count == -1 && process != null) { | ||
| if (!process.waitFor(shutdownTimeout.toNanos(), TimeUnit.NANOSECONDS)) { | ||
| throw new IOException( | ||
| "Subprocess stdout ended without process termination"); |
Comment on lines
+329
to
+330
| private IOException stopProcess() throws InterruptedException { | ||
| boolean exited = process.waitFor(processExitGrace.toNanos(), TimeUnit.NANOSECONDS); |
Comment on lines
+337
to
+338
| if (!process.waitFor(processExitGrace.toNanos(), TimeUnit.NANOSECONDS)) { | ||
| process.destroyForcibly(); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Deliver the R1 protocol-neutral transport foundations while preserving independent MCP, gRPC and Smithy SDK contracts. Stacked on #1556; this PR does not merge R0 or deliver the R2–R6 production extensions.
org.restlet.util.StreamTransport, backed by unexportedengine.stdio: on-demand bounded reads, serialized/flushed opaque writes, borrowed-stream cancellation and owned subprocess/pipes, separately drained bounded stderr, explicit startup/exit/I/O/cleanup failures.Request.getDisconnectSignal()independent of entity I/O, wrapper delegation and independent copied lifecycle. Remember cancellation before connector action registration; connector actions remain idempotent.Final-head follow-ups
The latest head afe0801 also fixes a deterministically reproduced client completion-publication gap: terminal state is now visible before body EOF wakes consumers. Added repeated immediate closes on empty 401/403 responses over HTTP/1.1 and HTTP/2 and included all eight existing authorization cases. Strict Javadoc and static-analysis findings were corrected without changing workflow or gate configuration.
Validation (Java 25)
whenServerIsHandlingBlockingRequestThenItRefusesNewRequest): 1.715 s versus 1 s +/- 200 ms. The identical test also failed on unchanged R0 (1.635 s); the entire five-case shutdown class passed in isolation on R1. No timing assertion or safeguard was relaxed. This is not a clean full-suite claim.git diff --checkpass. Reproduction command and all limitations are inreadme.md.Limits / ownership
Borrowed arbitrary input may ignore interruption: its owner must provide prompt cancellation. Stop reports a timeout rather than claiming cleanup if I/O is not released. Only owned process resources are terminated; process trees and borrowed connectors are not stopped. No physical-connection SPI is added; initial extension routing remains destination-URI-origin based.
Closes #1560 when the stacked work reaches the target release branch. Follow-up delivery: #1558, #1561, #1559.
Published checks
Verified head afe0801: repository build/test and strict Javadoc passed in CI run https://github.com/restlet/restlet-framework-java/actions/runs/39707048728. Both SonarCloud checks pass; quality gate OK and zero unresolved new-code issues. The branch is clean and synchronized; PR is ready for review and remains stacked on unmerged #1556.