ctubbsii commented on code in PR #3219:
URL: https://github.com/apache/accumulo/pull/3219#discussion_r1126657974
##########
core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/SeekableByteArrayInputStream.java:
##########
@@ -41,14 +44,15 @@ public class SeekableByteArrayInputStream extends
InputStream {
// thread 2 sees all of thread 1 changes before setting the volatile.
@SuppressFBWarnings(value = "VO_VOLATILE_REFERENCE_TO_ARRAY",
justification = "see explanation above")
- private volatile byte[] buffer;
- private int cur;
- private int max;
+ private final byte[] buffer;
Review Comment:
Yeah, if it's not volatile, then it shouldn't need the warnings suppression.
My concern, though, is that there's an attempt to explain that it was necessary
to have it volatile... but the explanation doesn't make sense to me, and I
can't see a reason why it should be volatile. I think making it final is
fine... but I still have reservations, and might want to try to do some digging
through the history to see if whoever wrote that can offer a better explanation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
