Skip to content

Comments

[FIXED] Reuse msg/hdr from pooled jsPubMsg#7790

Merged
neilalexander merged 1 commit intomainfrom
maurice/hdr-buf
Feb 6, 2026
Merged

[FIXED] Reuse msg/hdr from pooled jsPubMsg#7790
neilalexander merged 1 commit intomainfrom
maurice/hdr-buf

Conversation

@MauriceVanVeen
Copy link
Member

@MauriceVanVeen MauriceVanVeen commented Feb 3, 2026

Both the pm.buf and pm.hdr are meant to be reused, but the pm.hdr = nil above would prevent running pm.hdr = pm.hdr[:0]. Similarly, pm.msg also wasn't reused.

Signed-off-by: Maurice van Veen github@mauricevanveen.com
Co-authored-by: Neil Twigg neil@nats.io

@MauriceVanVeen MauriceVanVeen requested a review from a team as a code owner February 3, 2026 08:52
@MauriceVanVeen
Copy link
Member Author

Seems a data race was hiding here when reusing the hdr:

WARNING: DATA RACE
Write at 0x00c0029cf560 by goroutine 81975:
  runtime.slicecopy()
      /opt/hostedtoolcache/go/1.25.6/x64/src/runtime/slice.go:355 +0x0
  github.com/nats-io/nats-server/v2/server.newJSPubMsg()
      /home/runner/work/nats-server/nats-server/server/stream.go:6777 +0x155
  github.com/nats-io/nats-server/v2/server.(*consumer).releaseAnyPendingRequests()
      /home/runner/work/nats-server/nats-server/server/consumer.go:2851 +0x288
  github.com/nats-io/nats-server/v2/server.(*consumer).stopWithFlags()
      /home/runner/work/nats-server/nats-server/server/consumer.go:6071 +0x3f3
  github.com/nats-io/nats-server/v2/server.(*consumer).delete()
      /home/runner/work/nats-server/nats-server/server/consumer.go:6008 +0x3e
  github.com/nats-io/nats-server/v2/server.TestJetStreamWorkQueueMaxWaiting.func1.deferwrap3()
      /home/runner/work/nats-server/nats-server/server/jetstream_test.go:1125 +0x17

Previous read at 0x00c0029cf560 by goroutine 82001:
  runtime.growslice()
      /opt/hostedtoolcache/go/1.25.6/x64/src/runtime/slice.go:177 +0x0
  github.com/nats-io/nats-server/v2/server.(*stream).internalLoop()
      /home/runner/work/nats-server/nats-server/server/stream.go:6971 +0x18ac
  github.com/nats-io/nats-server/v2/server.(*stream).setupSendCapabilities.gowrap2()
      /home/runner/work/nats-server/nats-server/server/stream.go:6864 +0x33

@MauriceVanVeen MauriceVanVeen marked this pull request as draft February 3, 2026 09:18
@MauriceVanVeen
Copy link
Member Author

The race was due to reusing []byte("NATS/1.0 409 Consumer Deleted\r\n\r\n") for all released pull requests.
Have updated the PR to also reuse pm.msg, since only pm.buf was before, and pm.hdr was meant to be also.

@MauriceVanVeen MauriceVanVeen changed the title [FIXED] Reuse hdr from pooled jsPubMsg [FIXED] Reuse msg/hdr from pooled jsPubMsg Feb 3, 2026
@MauriceVanVeen MauriceVanVeen marked this pull request as ready for review February 3, 2026 10:22
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
Co-authored-by: Neil Twigg <neil@nats.io>
Copy link
Member

@neilalexander neilalexander left a comment

Choose a reason for hiding this comment

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

LGTM

@neilalexander neilalexander merged commit 046e91c into main Feb 6, 2026
69 of 70 checks passed
@neilalexander neilalexander deleted the maurice/hdr-buf branch February 6, 2026 17:09
neilalexander added a commit that referenced this pull request Feb 16, 2026
Includes the following:

- #7780
- #7784
- #7782
- #7783
- #7787
- #7789
- #7793
- #7797
- #7798
- #7799
- #7790
- #7805
- #7810
- #7811
- #7812
- #7809
- #7724
- #7815
- #7816
- #7818
- #7819
- #7820
- #7795
- #7825
- #7828
- #7835
- #7837

Signed-off-by: Neil Twigg <neil@nats.io>
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