Skip to content

fix(grpc): preserve subscription cleanup on disconnect - #509

Merged
yordis merged 14 commits into
masterfrom
yordis/fix-grpc-subscription-cleanup
Sep 24, 2026
Merged

yordis merged 14 commits into
masterfrom
yordis/fix-grpc-subscription-cleanup

Conversation

@yordis

@yordis yordis commented Sep 23, 2026 •

Copy link
Copy Markdown
Member
  • A disconnected client must not leave a live subscription behind or receive messages after disposal.

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@cursor

cursor Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes live subscription teardown ordering on the gRPC read path; incorrect sequencing could still leak subscriptions or drop events, but scope is limited to dispose logic with new regression tests.

Overview
Fixes a race where disposing a gRPC Read live subscription could leave server-side subscriptions registered if disposal happened while async startup was still in flight.

Stream, $all, and filtered $all enumerators now capture the background startup task from Subscribe, cancel on dispose, await that task (ignoring cancellation), and only then publish UnsubscribeFromStream. StreamSubscription also aligns dispose ordering by cancelling before awaiting startup.

Adds SubscriptionDisposalOrderingTests (delayed bus registration) and SubscriptionDisconnectTests (gRPC call dispose must emit unsubscribe) for all three subscription kinds.

Reviewed by Cursor Bugbot for commit fc98ac5. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Too many files!

This PR contains 115 files, which is 15 over the limit of 100.

To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch.

Upgrade to a paid plan to raise the limit.

This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.

Check out review usage here.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 4bc2b0f2-f722-4020-a15b-ba27b2cb5215

📥 Commits

Reviewing files that changed from the base of the PR and between 7fe01b9 and fc98ac5.

⛔ Files ignored due to path filters (1)
  • proto.lock is excluded by !**/*.lock
📒 Files selected for processing (115)
  • src/EventStore.ClusterNode/Components/Pages/Cluster.razor
  • src/EventStore.ClusterNode/Components/Services/ClusterStatusService.cs
  • src/EventStore.Core.Tests/ClientAPI/Helpers/EventDataComparer.cs
  • src/EventStore.Core.Tests/ClientAPI/Helpers/EventsStream.cs
  • src/EventStore.Core.Tests/ClientAPI/Helpers/TcpType.cs
  • src/EventStore.Core.Tests/ClientAPI/Helpers/TestConnection.cs
  • src/EventStore.Core.Tests/ClientAPI/Helpers/TestConnectionLifecycle.cs
  • src/EventStore.Core.Tests/ClientAPI/Helpers/TestEvent.cs
  • src/EventStore.Core.Tests/ClientAPI/Helpers/Writer.cs
  • src/EventStore.Core.Tests/ClientAPI/SpecificationWithMiniNode.cs
  • src/EventStore.Core.Tests/Cluster/MemberInfoTests.cs
  • src/EventStore.Core.Tests/DefaultData.cs
  • src/EventStore.Core.Tests/EventStore.Core.Tests.csproj
  • src/EventStore.Core.Tests/Helpers/ClientApiLoggerBridge.cs
  • src/EventStore.Core.Tests/Helpers/MiniClusterNode.cs
  • src/EventStore.Core.Tests/Helpers/MiniNode.cs
  • src/EventStore.Core.Tests/Integration/Archive/when_archiving_and_restoring_a_cluster.cs
  • src/EventStore.Core.Tests/Integration/specification_with_cluster.cs
  • src/EventStore.Core.Tests/Integration/when_node_becomes_leader_with_unindexed_data.cs
  • src/EventStore.Core.Tests/Regression/ClusterStatusServiceTests.cs
  • src/EventStore.Core.Tests/Services/ElectionsService/ClusterSettingsFactory.cs
  • src/EventStore.Core.Tests/Services/ElectionsService/ClusterVNodeSettings.cs
  • src/EventStore.Core.Tests/Services/ElectionsService/ElectionServiceUnit.cs
  • src/EventStore.Core.Tests/Services/ElectionsService/ElectionsServiceTests.cs
  • src/EventStore.Core.Tests/Services/ElectionsService/LeaderNode/ElectionsServiceUnitTests.cs
  • src/EventStore.Core.Tests/Services/ElectionsService/Randomized/RandomizedElectionsTestCase.cs
  • src/EventStore.Core.Tests/Services/ElectionsService/Randomized/UpdateGossipProcessor.cs
  • src/EventStore.Core.Tests/Services/ElectionsService/Randomized/elections_service_5_nodes_with_1_known_when_started_and_set_full_imediately.cs
  • src/EventStore.Core.Tests/Services/ElectionsService/Randomized/elections_service_5_nodes_with_1_known_when_started_and_set_to_full_later.cs
  • src/EventStore.Core.Tests/Services/GossipService/NodeGossipServiceTests.cs
  • src/EventStore.Core.Tests/Services/Replication/LogReplication/LogReplicationFixture.cs
  • src/EventStore.Core.Tests/Services/Replication/ReadOnlyReplica/connecting_to_read_only_replica.cs
  • src/EventStore.Core.Tests/Services/RequestForwarding/GrpcRequestForwardingServiceTests.cs
  • src/EventStore.Core.Tests/Services/RequestForwarding/GrpcRequestForwardingTransportSecurityTests.cs
  • src/EventStore.Core.Tests/Services/RequestForwarding/RequestForwardingServiceTests.cs
  • src/EventStore.Core.Tests/Services/RequestManagement/Service/when_writing_and_deposed_as_leader.cs
  • src/EventStore.Core.Tests/Services/RequestManagement/Service/when_writing_and_deposed_as_leader_and_replica_moves_forward.cs
  • src/EventStore.Core.Tests/Services/Transport/Enumerators/SubscriptionDisposalOrderingTests.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Forwarding/ForwardingGrpcCodecTests.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/Replication/GrpcReplicaServiceSupervisorTests.cs
  • src/EventStore.Core.Tests/Services/Transport/Grpc/StreamsTests/SubscriptionDisconnectTests.cs
  • src/EventStore.Core.Tests/Services/Transport/Tcp/core_tcp_package.cs
  • src/EventStore.Core.Tests/Services/Transport/Tcp/ssl_connection.cs
  • src/EventStore.Core.Tests/Services/Transport/Tcp/ssl_connections_mutual_auth.cs
  • src/EventStore.Core.Tests/Services/VNode/InaugurationManager/InaugurationManagerTests.cs
  • src/EventStore.Core.Tests/Services/VNode/ShutdownServiceTests.cs
  • src/EventStore.Core.Tests/Services/VNode/leader_info_provider.cs
  • src/EventStore.Core.Tests/TcpApiTestPlugin/PublicTcpApiTestService.cs
  • src/EventStore.Core.Tests/TcpApiTestPlugin/TcpApiTestOptions.cs
  • src/EventStore.Core.Tests/TcpApiTestPlugin/TcpApiTestPlugin.cs
  • src/EventStore.Core.Tests/TransactionLog/Truncation/when_truncating_database.cs
  • src/EventStore.Core.XUnit.Tests/Configuration/ClusterNodeOptionsTests/when_building/with_cluster_node_and_custom_settings.cs
  • src/EventStore.Core.XUnit.Tests/Configuration/ClusterNodeOptionsTests/when_building/with_default_settings.cs
  • src/EventStore.Core.XUnit.Tests/Metrics/ElectionsCounterTrackerTests.cs
  • src/EventStore.Core.XUnit.Tests/Services/Storage/InMemory/GossipListenerServiceTests.cs
  • src/EventStore.Core.XUnit.Tests/Telemetry/TelemetryServiceTests.cs
  • src/EventStore.Core/Authorization/AuthorizationPolicies/LegacyPolicySelectorFactory.cs
  • src/EventStore.Core/Cluster/ClientClusterInfo.cs
  • src/EventStore.Core/Cluster/ClusterInfo.cs
  • src/EventStore.Core/Cluster/MemberInfo.cs
  • src/EventStore.Core/ClusterVNode.cs
  • src/EventStore.Core/Configuration/ClusterVNodeOptions.cs
  • src/EventStore.Core/Configuration/ClusterVNodeOptionsExtensions.cs
  • src/EventStore.Core/Data/GossipAdvertiseInfo.cs
  • src/EventStore.Core/Data/VNodeInfo.cs
  • src/EventStore.Core/EventStore.Core.csproj
  • src/EventStore.Core/Helpers/IAsyncMessageFramer.cs
  • src/EventStore.Core/Helpers/LengthPrefixSuffixFramer.cs
  • src/EventStore.Core/Helpers/MessageFramingException.cs
  • src/EventStore.Core/Messages/ClientMessage.cs
  • src/EventStore.Core/Messages/ClusterInfoDto.cs
  • src/EventStore.Core/Messages/MemberInfoDto.cs
  • src/EventStore.Core/Messages/TcpClientMessageDto.cs
  • src/EventStore.Core/Messages/TcpMessage.cs
  • src/EventStore.Core/Messaging/CoreMessage.cs
  • src/EventStore.Core/NodeTcpOptions.cs
  • src/EventStore.Core/Services/AuthorizationGateway.cs
  • src/EventStore.Core/Services/ElectionsService.cs
  • src/EventStore.Core/Services/Gossip/GossipServiceBase.cs
  • src/EventStore.Core/Services/Gossip/NodeGossipService.cs
  • src/EventStore.Core/Services/Monitoring/MonitoringService.cs
  • src/EventStore.Core/Services/Monitoring/SystemStatsHelper.cs
  • src/EventStore.Core/Services/PersistentSubscription/PersistentSubscriptionService.cs
  • src/EventStore.Core/Services/Replication/LogRecordFramer.cs
  • src/EventStore.Core/Services/Replication/TransactionFramer.cs
  • src/EventStore.Core/Services/RequestForwarding/GrpcRequestForwardingSupervisor.cs
  • src/EventStore.Core/Services/RequestForwardingService.cs
  • src/EventStore.Core/Services/Storage/ReaderIndex/EventFilter.cs
  • src/EventStore.Core/Services/SubscriptionsService.cs
  • src/EventStore.Core/Services/TcpSendService.cs
  • src/EventStore.Core/Services/Transport/Enumerators/Enumerator.AllSubscription.cs
  • src/EventStore.Core/Services/Transport/Enumerators/Enumerator.AllSubscriptionFiltered.cs
  • src/EventStore.Core/Services/Transport/Enumerators/Enumerator.StreamSubscription.cs
  • src/EventStore.Core/Services/Transport/Grpc/Forwarding/ForwardingGrpcCodec.cs
  • src/EventStore.Core/Services/Transport/Grpc/Forwarding/ForwardingService.cs
  • src/EventStore.Core/Services/Transport/Tcp/ClientTcpDispatcher.cs
  • src/EventStore.Core/Services/Transport/Tcp/ClientWriteTcpDispatcher.cs
  • src/EventStore.Core/Services/Transport/Tcp/ITcpDispatcher.cs
  • src/EventStore.Core/Services/Transport/Tcp/ProtobufExtensions.cs
  • src/EventStore.Core/Services/Transport/Tcp/SendOverTcpEnvelope.cs
  • src/EventStore.Core/Services/Transport/Tcp/TcpCommand.cs
  • src/EventStore.Core/Services/Transport/Tcp/TcpConnectionManager.cs
  • src/EventStore.Core/Services/Transport/Tcp/TcpDispatcher.cs
  • src/EventStore.Core/Services/Transport/Tcp/TcpPackage.cs
  • src/EventStore.Core/Services/Transport/Tcp/TcpService.cs
  • src/EventStore.Core/Services/Transport/Tcp/UnableToAcquireStreamException.cs
  • src/EventStore.Core/Services/VNode/ClusterVNodeController.cs
  • src/EventStore.Core/Services/VNode/LeaderInfoProvider.cs
  • src/EventStore.Core/Settings/VNodeSettings.cs
  • src/EventStore.Core/Util/Opts.cs
  • src/EventStore.Projections.Core.Tests/ClientAPI/Cluster/specification_with_standard_projections_runnning.cs
  • src/EventStore.Projections.Core.Tests/Services/projections_system/when_starting_up.cs
  • src/Protos/ClientAPI/ClientMessageDtos.proto
  • src/Protos/Grpc/cluster.proto
  • src/Protos/Grpc/forwarding.proto

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@yordis yordis changed the title yordis/fix grpc subscription cleanup fix(grpc): preserve subscription cleanup on disconnect Sep 23, 2026
@yordis
yordis marked this pull request as ready for review September 23, 2026 16:13
@yordis
yordis requested a review from a team as a code owner September 23, 2026 16:13
Base automatically changed from yordis/chore-remove-legacy-tcp-runtime to master September 24, 2026 01:14
@yordis
yordis merged commit d30db04 into master Sep 24, 2026
36 checks passed
@yordis
yordis deleted the yordis/fix-grpc-subscription-cleanup branch September 24, 2026 01:15
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.

1 participant