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 (which can also be removed if it's not 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]
