Skip to content
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.

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

Possible dereference #7765

Closed
Bbulatov opened this issue Oct 21, 2024 · 1 comment
Closed

Possible dereference #7765

Bbulatov opened this issue Oct 21, 2024 · 1 comment
Assignees
Labels
Area: Client Includes Channel/Subchannel/Streams, Connectivity States, RPC Retries, Dial/Call Options and more. Type: Bug

Comments

@Bbulatov
Copy link

Hello!
During the static analysis was found possible mistake:

  1. After having been compared to a nil value at stream.go:1029, pointer 'cs.attempt' is passed in call to function 'grpc.clientStream.Trailer' at stream.go:1053, where it is dereferenced at stream.go:858.
default:
			logEntry := &binarylog.ServerTrailer{
				OnClientSide: true,
				Trailer:      cs.Trailer(),
				Err:          err,
			}

version of gRPC: master
version of Go: go version go1.22.4 linux/amd64
operating system: Linux debian 10

@aranjans
Copy link
Contributor

Thanks for raising this issue and bringing it to our attention. The static analysis you're using might be flagging a false positive here.

Let's break down why:

  • cs.attempt initialization: The cs.attempt field is initially nil, but it's populated within the newClientStreamWithParams function before being used in any critical operations.

  • Guaranteed non-nil in Trailer(): The Trailer() method is called only after the stream is considered finished, either successfully or due to an error. By this point, cs.attempt is guaranteed to be non-nil. This is ensured because:

    • A successful stream creation would have assigned a value to cs.attempt.
    • An error during stream creation would lead to the finish method being called, which handles the case of cs.attempt potentially being nil.
    • finish() method's safeguard: The finish() method itself includes a check (if cs.attempt != nil) to prevent nil pointer dereferences in scenarios where the stream might not have been fully established.

If you can provide a minimal, reproducible example that demonstrates the issue, that would be even more helpful in diagnosing the problem.

@aranjans aranjans self-assigned this Oct 22, 2024
@purnesh42H purnesh42H added the Area: Client Includes Channel/Subchannel/Streams, Connectivity States, RPC Retries, Dial/Call Options and more. label Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Client Includes Channel/Subchannel/Streams, Connectivity States, RPC Retries, Dial/Call Options and more. Type: Bug
Projects
None yet
Development

No branches or pull requests

3 participants