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

fix(DBInstance): handle cases where both previous and desired allocated storage are null #557

Merged
merged 1 commit into from
Jul 12, 2024

Conversation

kylenie-aws
Copy link
Contributor

This change addresses an issue causing DB instance updates to fail during rollback with an InternalFailure. The error message was "Cannot invoke 'java.lang.Integer.intValue()' because 'b' is null". The fix ensures the max() function can handle null inputs.

.build();
final ResourceModel desiredModel = RESOURCE_MODEL_BLDR()
.allocatedStorage(null)
.storageType(STORAGE_TYPE_IO1)
Copy link

Choose a reason for hiding this comment

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

is this required in the test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kylenie-aws kylenie-aws added this pull request to the merge queue Jul 12, 2024
Merged via the queue into aws-cloudformation:master with commit 440766b Jul 12, 2024
1 check passed
@kylenie-aws kylenie-aws deleted the fix-max-funtion branch July 12, 2024 02:13
@zrfr
Copy link
Collaborator

zrfr commented Jul 18, 2024

This bug fix is completely inscrutable and took me some time to figure out.

For the benefit of anyone else who comes across this change: the bug was that the max function crashed with an NPE when both of the arguments were null. The cause was due to how Java's conditional/ternary expression handles types (see JLS). Java determines the type of a conditional expression based on the second and third operands. If one operand is a primitive type and the other is a boxed number, then the expression's type is primitive, requiring the boxed operand to be unboxed. This crashes if the operand is null. The fix works by forcing the expression's type to be boxed, which avoids the unboxing conversion.

I didn't manage to review this before we merged it, but I would have preferred a more understandable fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants