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

why not call cancel() after context.WithTimeout, is it a bug or have something another? #1099

Closed
liangzhiyang opened this issue Mar 2, 2017 · 2 comments

Comments

@liangzhiyang
Copy link

https://github.com/grpc/grpc-go/blob/master/clientconn.go line 773
// have none defer cancel()
ctx, cancel := context.WithTimeout(ac.ctx, timeout)
...
newTransport, err := transport.NewClientTransport(ctx, sinfo, ac.dopts.copts)
if err != nil {
cancel()
```
}
//if err == nil , cancel() won't been called

but func WithTimeout docs here:
// WithTimeout returns WithDeadline(parent, time.Now().Add(timeout)).
//
// Canceling this context releases resources associated with it, so code should
// call cancel as soon as the operations running in this Context complete:
//
// func slowOperationWithTimeout(ctx context.Context) (Result, error) {
// ctx, cancel := context.WithTimeout(ctx, 100*time.Millisecond)
// defer cancel() // releases resources if slowOperation completes before timeout elapses
// return slowOperation(ctx)
// }

is it a bug or have something another??

@dfawley
Copy link
Member

dfawley commented Mar 4, 2017

Nice catch. You're right -- normally it would be safe to cancel the context immediately after the client is created. Unfortunately, making that change stimulates a bug in Go 1.6's implementation of Dial that results in subsequent I/O errors: golang/go#15078

Not cancelling is not a major problem, because the resources will eventually be freed after the context leaves scope, but it's unusual enough that I'll add a comment to the code explaining why we aren't calling it.

@liangzhiyang
Copy link
Author

thanks

@lock lock bot locked as resolved and limited conversation to collaborators Sep 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants