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

Synchronize ByteSourcePayload in LocalBlobStore #146

Closed
wants to merge 1 commit into from

Conversation

gaul
Copy link
Member

@gaul gaul commented Jul 24, 2022

ByteSourcePayload's.openStream is not thread safe and lack of
synchronization can throw ArrayIndexOutOfBoundsExceptions.
Fixes gaul/s3proxy#303.

@gaul gaul requested a review from nacx July 24, 2022 12:27
@gaul
Copy link
Member Author

gaul commented Jul 24, 2022

I think there is a narrower fix where LocalBlobStore could synchronize its calls sites instead. @nacx what do you think?

Copy link
Member

@nacx nacx left a comment

Choose a reason for hiding this comment

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

LGTM!

@nacx
Copy link
Member

nacx commented Jul 25, 2022

If the LocalBlobStore can sync ina. narrower scope that would be much better

ByteSourcePayload's.openStream is not thread safe and lack of
synchronization can throw ArrayIndexOutOfBoundsExceptions.
Fixes gaul/s3proxy#303.
@gaul gaul changed the title Synchronize ByteSourcePayload Closer methods Synchronize ByteSourcePayload in LocalBlobStore Jul 30, 2022
@gaul
Copy link
Member Author

gaul commented Jul 30, 2022

If the LocalBlobStore can sync ina. narrower scope that would be much better

Done.

@gaul
Copy link
Member Author

gaul commented Jul 30, 2022

Something seems strange here -- every call to getBlob should return a new Payload...

@gaul
Copy link
Member Author

gaul commented Aug 1, 2022

Superseded by #150.

@gaul gaul closed this Aug 1, 2022
@gaul gaul deleted the synchronized-closer branch August 3, 2022 12:56
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.

intermittent getBlob exception
2 participants