Repository navigation
Remove synchronization in HTTP/1 server request and response implementations - #6438
Merged
Merged
Conversation
Motivation: Http1ServerRequest and Http1ServerResponse fields are accessed from the event loop, the context thread or both, which is hard to reason about and is the basis for removing the connection lock. Modifications: Order the fields by owner thread (immutable, written on the event loop before the request handler, event loop, event loop and context thread, context thread) with a header per group. The WebSocket handshake no longer replaces the request field on the context thread: the full request is passed to Http1ServerConnection.createWebSocket and ServerWebSocketHandshaker instead, so the request field is final. Assisted-by: Anthropic Claude (claude-opus-5-5) Signed-off-by: Julien Viet <julien@julienviet.com>
… expected thread and context Motivation: The WebSocket handshake completion and the send file completion ran on the event loop and updated or read response state owned by the context thread. The send file body end and end handlers were also called on the caller context instead of the response context. Modifications: handleHandshakeCompleted (event loop) and the send file completion listener post their work to the response context, where the response state is updated, the body end and end handlers are dispatched and the close is performed. Assisted-by: Anthropic Claude (claude-opus-5-5) Signed-off-by: Julien Viet <julien@julienviet.com>
Motivation: createWebSocket is called on the request context thread and wrote the connection wantClose field, owned by the event loop, before posting the rest of its work to the event loop. Modifications: Reset wantClose inside the task executed on the event loop. Assisted-by: Anthropic Claude (claude-opus-5-5) Signed-off-by: Julien Viet <julien@julienviet.com>
Motivation: Http1ServerRequest flushed the connection bytes read counter, owned by the event loop, from the request context thread when the request end was delivered. Modifications: Flush it in Http1ServerConnection.onEnd on the event loop when the request end is received. Assisted-by: Anthropic Claude (claude-opus-5-5) Signed-off-by: Julien Viet <julien@julienviet.com>
Motivation: Http1ServerRequest and Http1ServerResponse synchronized on the connection to protect state that is now owned by a single thread: each field is written either on the event loop before the request handler is called, or on the request context thread. Cross-thread hand-offs already rely on emit/execute and the connection write path. Modifications: Remove synchronized (conn) and the synchronized methods from Http1ServerRequest and Http1ServerResponse. The inbound queue keeps its double-checked creation. The request and response API must be used from the request context thread; other threads are not supported. Assisted-by: Anthropic Claude (claude-opus-5-5) Signed-off-by: Julien Viet <julien@julienviet.com>
… lock is gone Motivation: The headers end handler was taken and called before preparing the head, in separate steps, only to call it outside the connection lock, which has been removed. Modifications: prepareHeaders calls the headers end handler first again, and the head writing methods are single blocks. Each caller sets headersSent when it sends the head. Assisted-by: Anthropic Claude (claude-opus-5-5) Signed-off-by: Julien Viet <julien@julienviet.com>
Motivation: Some temporaries in Http1ServerResponse were only needed while state was read under the connection lock. Modifications: Read the end handlers and the request end state directly after writing the last message, and simplify onHandshakeCompleted and the handler setters. Assisted-by: Anthropic Claude (claude-opus-5-5) Signed-off-by: Julien Viet <julien@julienviet.com>
vietj
force-pushed
the
server-impl-sync
branch
from
October 15, 2026 15:25
3da55e6 to
04ef9a6
Compare
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
1 participant
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.
HTTP/1.1 server request and response implementation is using a synchronization on the connection object. This should not be necessary since the we introduced a clean separation and message passing between the event-loop of the connection and the context of the stream.
This contribution first clarifies state access of the request and response and ensures that field access from the various parts is correct.
Under that assumption, synchronization on the connection object is only required by state access from the context thread(s). We remove this synchronization, based on the hypothesis that it used to be important when state patterns were not clarified and message passing was not always enforced as invariants.