Skip to content

Fix a race in FileSessionCredentialsProvider#6664

Merged
leventov merged 3 commits intoapache:masterfrom
metamx:aws-file-creds-privider-race
Nov 27, 2018
Merged

Fix a race in FileSessionCredentialsProvider#6664
leventov merged 3 commits intoapache:masterfrom
metamx:aws-file-creds-privider-race

Conversation

@leventov
Copy link
Copy Markdown
Member

sessionToken, accessKey and secretKey must be updated atomically.

Another race is possible between the file updater and the Druid process reading the file. It could be enforced only with mandatory file locking, but file locking is advisory by default in Linux.

@leventov leventov added the Bug label Nov 27, 2018
@leventov
Copy link
Copy Markdown
Member Author

An issue about reader/writer race: #6665

@leventov
Copy link
Copy Markdown
Member Author

This issue might be a tip of an iceberg: #6666

Copy link
Copy Markdown
Contributor

@jihoonson jihoonson left a comment

Choose a reason for hiding this comment

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

LGTM

private volatile String accessKey;
private volatile String secretKey;
private final String sessionCredentialsFile;
private AWSSessionCredentials awsSessionCredentials;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry for leaving a comment after LGTM, but is this needed to be volatile?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It doesn't need to be volatile. From JMM point of view, volatile on this field changes anything and doesn't provide any extra guarantees.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll leave this as a comment in code though.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Right, thanks.

@leventov leventov merged commit b4a4669 into apache:master Nov 27, 2018
@leventov leventov deleted the aws-file-creds-privider-race branch November 27, 2018 20:02
@jon-wei jon-wei added this to the 0.14.0 milestone Feb 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants