Skip to content

fix: add missing yield break after shutdown response in JsonRpcProcessor #10462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
LukaszRozmej merged 5 commits into from
Feb 9, 2026
Merged

fix: add missing yield break after shutdown response in JsonRpcProcessor #10462

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f7a8d21
fix: add missing yield break after shutdown response
VolodymyrBg Feb 8, 2026
89ff44f
Add test
VolodymyrBg Feb 8, 2026
0bfdd6d
ensure pipereader is completed on shutdown early-exit
VolodymyrBg Feb 9, 2026
75c0925
cover pipereader completion when shutdown is requested
VolodymyrBg Feb 9, 2026
057c085
Merge branch 'master' into fix/missing-yield-break-on-shutdown
LukaszRozmej Feb 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions src/Nethermind/Nethermind.JsonRpc.Test/JsonRpcProcessorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Threading.Tasks;
using FluentAssertions;
using Nethermind.Core.Extensions;
using Nethermind.Config;
using Nethermind.Logging;
using Nethermind.JsonRpc.Modules;
using NSubstitute;
Expand Down Expand Up @@ -401,6 +402,33 @@ public async Task Can_handle_null_request()
result.DisposeItems();
}

[Test]
public async Task Should_stop_processing_when_shutdown_requested()
{
IJsonRpcService service = Substitute.For<IJsonRpcService>();
service.GetErrorResponse(Arg.Any<int>(), Arg.Any<string>())
.Returns(new JsonRpcErrorResponse { Error = new Error { Code = ErrorCodes.ResourceUnavailable, Message = "Shutting down" } });

IProcessExitSource processExitSource = Substitute.For<IProcessExitSource>();
processExitSource.Token.Returns(new CancellationToken(canceled: true));

JsonRpcProcessor processor = new(
service,
new JsonRpcConfig(),
Substitute.For<IFileSystem>(),
LimboLogs.Instance,
processExitSource);

string request = "{\"id\":67,\"jsonrpc\":\"2.0\",\"method\":\"eth_getTransactionCount\",\"params\":[\"0x7f01d9b227593e033bf8d6fc86e634d27aa85568\",\"0x668c24\"]}";
List<JsonRpcResult> results = await processor.ProcessAsync(request, new JsonRpcContext(RpcEndpoint.Http)).ToListAsync();

results.Should().HaveCount(1);
results[0].Response.Should().BeOfType<JsonRpcErrorResponse>();
((JsonRpcErrorResponse)results[0].Response!).Error!.Code.Should().Be(ErrorCodes.ResourceUnavailable);
await service.DidNotReceive().SendRequestAsync(Arg.Any<JsonRpcRequest>(), Arg.Any<JsonRpcContext>());
results.DisposeItems();
}

[Test]
public void Cannot_accept_null_file_system()
{
Expand Down
1 change: 1 addition & 0 deletions src/Nethermind/Nethermind.JsonRpc/JsonRpcProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ public async IAsyncEnumerable<JsonRpcResult> ProcessAsync(PipeReader reader, Jso
{
JsonRpcErrorResponse response = _jsonRpcService.GetErrorResponse(ErrorCodes.ResourceUnavailable, "Shutting down");
yield return JsonRpcResult.Single(RecordResponse(response, new RpcReport("Shutdown", 0, false)));
yield break;
}

Copilot AI Feb 9, 2026

Copy link

Choose a reason for hiding this comment

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

The new yield break exits the async iterator before reaching the finally { await reader.CompleteAsync(); } block, so the PipeReader is never completed when shutdown is requested. This changes the previous ownership/cleanup behavior and can leave pipes/connections uncompleted. Consider restructuring so the shutdown early-exit is inside a try/finally that always completes the reader (e.g., wrap the whole method body in an outer try with await reader.CompleteAsync() in finally, or move the shutdown check inside the existing try scope).

Copilot uses AI. Check for mistakes.

@LukaszRozmej LukaszRozmej Feb 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot open a new pull request to apply changes based on this feedback


if (IsRecordingRequest)
Expand Down
Loading