-
Notifications
You must be signed in to change notification settings - Fork 3
Fix NPE due to concurrent access #138
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
Conversation
92bd4da to
600eb81
Compare
| * */ | ||
| @Override | ||
| public void configureClient(SdkServiceClientConfiguration.Builder config) { | ||
| ClientOverrideConfiguration overrideConfig = ClientOverrideConfiguration.builder() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: consider moving closer to usage (line 105).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack
| accessGrantsPlugin.configureClient(config); | ||
| successCount.incrementAndGet(); | ||
| } catch (Exception e) { | ||
| exceptionCount.incrementAndGet(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: log exception for easier test failure troubleshooting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
LGTM! Tests are also passing. Any failures in automation tests are due to dependency graph update problems and will not be related to this PR. |
Description of changes:
The exception occurs in the AWS SDK's internal AttributeMap.Builder.resolveValue() method at line 396, which validates that configuration attributes
are not null. The stack trace shows:
java.lang.NullPointerException: Encountered a null value when resolving configuration attributes.
This is commonly caused by concurrent modifications to non-thread-safe types.
The Threading Problem
Original Code Structure:
What Happens During Concurrent Access:
Internal AWS SDK Flow
The AttributeMap Threading Issue
The AttributeMap.Builder maintains internal collections that are modified during configuration resolution:
When multiple threads modify this map concurrently, the HashMap's internal structure becomes inconsistent, leading to null values being returned for
valid keys.