Skip to content

fix(pubsub/v2): prevent nil span panic in Subscriber.Receive#14278

Merged
hongalex merged 1 commit intogoogleapis:mainfrom
fredrikaverpil:fix/pubsub-v2-nil-span-panic
Mar 31, 2026
Merged

fix(pubsub/v2): prevent nil span panic in Subscriber.Receive#14278
hongalex merged 1 commit intogoogleapis:mainfrom
fredrikaverpil:fix/pubsub-v2-nil-span-panic

Conversation

@fredrikaverpil
Copy link
Copy Markdown
Contributor

@fredrikaverpil fredrikaverpil commented Mar 27, 2026

Why?

When tracing is enabled on a pubsub v2 subscriber, there is a race between the subscriber loop and the ack/nack/expiry paths. activeSpans is a sync.Map shared across goroutines, and the span can be deleted between the Store (message received in iter.receive) and the Load (message dispatched in Subscriber.Receive):

  1. iter.receive() stores the subscribe span in activeSpans (iterator.go:382)
  2. Before the subscriber loop reads it, one of these deletes the span concurrently:
    • Message expiry: keepAliveDeadlines cleanup calls activeSpans.LoadAndDelete (iterator.go:582)
    • Ack: sendAck calls activeSpans.LoadAndDelete (iterator.go:659)
    • Nack: sendModAck with isNack=true calls activeSpans.LoadAndDelete (iterator.go:739)
  3. activeSpans.Load(ackh.ackID) returns ok=false, so ccSpan stays nil (subscriber.go:408)
  4. ccSpan.End() panics on nil (subscriber.go:427)

What?

Add a nil check before calling ccSpan.End(). If the parent subscribe span was already removed from activeSpans, there is nothing to link the concurrency control span to — skipping it seems like the correct semantic behavior.

Notes

The issue reporter (@magnusewe) noted "this started happening after the client had been running for a while", which aligns with message expiry being the most likely trigger.

Fixes #14277

When tracing is enabled, the concurrency control span (ccSpan) is only
assigned if the ack ID is found in activeSpans. However, ccSpan.End()
was called unconditionally when tracing is enabled, causing a nil
pointer panic when the ack ID was not in the map.

Add a nil check before calling ccSpan.End() to prevent the panic.

Fixes googleapis#14277
@fredrikaverpil fredrikaverpil requested review from a team as code owners March 27, 2026 12:26
@product-auto-label product-auto-label Bot added the api: pubsub Issues related to the Pub/Sub API. label Mar 27, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds a null check for ccSpan in the Receive method of the subscriber. The review feedback suggests simplifying the conditional check by removing the redundant iter.enableTracing flag, as the null check for ccSpan is sufficient.

Comment thread pubsub/v2/subscriber.go
@hongalex hongalex added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 31, 2026
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 31, 2026
@hongalex hongalex merged commit c590c35 into googleapis:main Mar 31, 2026
10 of 11 checks passed
hongalex added a commit that referenced this pull request Mar 31, 2026
PR created by the Librarian CLI to initialize a release. Merging this PR
will auto trigger a release.

Librarian Version: v0.8.3
Language Image:
us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/librarian-go@sha256:f8558b849eab319c3b9fc0b0ee8cab738b7e0b88e746275c759b38b194247a42
<details><summary>pubsub/v2: v2.5.1</summary>

##
[v2.5.1](pubsub/v2.5.0...pubsub/v2.5.1)
(2026-03-31)

### Bug Fixes

* prevent nil span panic in Subscriber.Receive (#14278)
([c590c35](c590c350))

</details>
fredrikaverpil added a commit to einride/cloudrunner-go that referenced this pull request Apr 1, 2026
fredrikaverpil added a commit to einride/cloudrunner-go that referenced this pull request Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: pubsub Issues related to the Pub/Sub API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pubsub: panic when using tracing due to nil Span interface

4 participants