Skip to content

Release 2.8.2#1189

Merged
mtmk merged 1 commit into
mainfrom
release-2.8.2
Jun 17, 2026
Merged

Release 2.8.2#1189
mtmk merged 1 commit into
mainfrom
release-2.8.2

Conversation

@mtmk

@mtmk mtmk commented Jun 16, 2026

Copy link
Copy Markdown
Member

Patch release with JetStream teardown fixes and a dependency bump.

@synadia-claude-reviewer

synadia-claude-reviewer Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Claude finished @mtmk's task in 1m 50s —— View job


PR Review: Release 2.8.2

  • Gather context and read PR details
  • Examine PR diff
  • Review changed files and related code
  • Post review summary

Review Summary

This is a clean patch release PR. The only diff in this PR itself is the version bump in version.txt (2.8.1 → 2.8.2), which is correct for a patch release. The substantive changes landed via the referenced PRs already merged to main.

JetStream subscription leak fix (#1188)NatsJSOrderedPushConsumer.cs

The fix is correct and well-structured. DisposeAsync now disposes the current _sub after both background loops (_consumerCreateTask, _commandTask) have completed:

var sub = _sub;
if (sub != null)
{
    await sub.UnsubscribeAsync();
    await sub.DisposeAsync();
}
  • Thread safety: _sub is volatile; reading it into a local after both tasks complete is safe — no new subscription can be assigned at that point.
  • Ordering: Completing the channels (TryComplete) and awaiting both tasks before touching _sub is the right order. This matches the existing pattern in CreatePushConsumer (lines 374–375).
  • Event handler cleanup: NatsJSOrderedPushConsumerSub.DisposeAsync already unhooks ConnectionOpened (line 491), so the root cause of the leak is properly severed.
  • One minor note: _consumerCreateChannel.Writer.TryComplete() is awaited via _consumerCreateTask, but there's no await on the TryComplete calls themselves — this is fine since TryComplete is synchronous and the subsequent await on the tasks ensures the loops have fully drained.

MessagePack 3.1.1 → 3.1.7 (#1183): routine dependency bump, no concerns.

DI package description clarifications (#1163): docs-only, no code impact.

Overall: The release is sound. No correctness, security, or performance concerns.

@Jarema Jarema left a comment

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.

LGTM

@mtmk
mtmk merged commit 9791b16 into main Jun 17, 2026
25 checks passed
@mtmk
mtmk deleted the release-2.8.2 branch June 17, 2026 10:16
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