-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Goroutine leak in credentials GetWithContext #3606
Comments
Thanks for reaching out @rittneje about the leaded goroutine. I'm going to review the change, and see how we can fix it to prevent or reduce the chance of leaking the goroutine. One of the issues that we want to solve is that the underlying provider is not safe to be called concurrently. Since the v1 SDK's design has multiple entry points into the provider(Retrieve and IsExpired), we're seeing the conflict with trying to refresh the credentials, and check if they are expired at the same time on the provider. This is the reasoning behind having the the Read Write lock being used by the |
This issue has been fixed in aws/aws-sdk-go-v2. |
|
The fix for #3524 (#3448) introduces a goroutine to call
provider.IsExpired()
. However, if the call doesn't return in a timely manner, that goroutine will be leaked when the context expires.The prior behavior of the SDK was to block indefinitely on the call the
IsExpired
, and I don't see a reason to change that behavior. I propose removing the newly addedasyncIsExpired
helper function and callingIsExpired
directly while holding the read lock, as I did in my fix (#3563) to avoid this very problem.If allowing the call to
IsExpired
to terminate early is desirable, then I propose adding an optionalIsExpiredWithContext
method toProvider
. If the provider doesn't implement this method, then context cancellation will not be fully honored. This is analogous to how the database/sql package handles a similar issue with drivers.The text was updated successfully, but these errors were encountered: