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
read() in inputstream should return unsigned value #370
Conversation
@@ -2262,7 +2262,8 @@ object ByteVector extends ByteVectorCompanionCrossPlatform { | |||
private val bvlen = bv.size.toInt | |||
private val pos = new CustomAtomicInteger(0) | |||
|
|||
override def read(): Int = bv.get(pos.getAndIncrement().toLong) | |||
override def read(): Int = | |||
if (pos.get() >= bvlen) -1 else bv.get(pos.getAndIncrement().toLong) & 0xff |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Safer to get current value of pos
once instead of calling it twice here.
val p = pos.getAndIncrement()
if (p >= bvlen) -1
else bv.get(p.toLong) & 0xff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it is a good idea, since available
then will return negative values if read()
is called at the end of the stream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, my concern is with thread safety - a race between concurrent read calls and an interleaving of get/get/getAndIncrement/getAndOncrement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see. So a fix could be in available()
instead I guess.
Thanks for spotting & fixing. Can we add a unit test for this as well? |
@mpilquist I have added a small test. |
Fixes #369
read()
should return a value between 0 and 255 (https://docs.oracle.com/javase/9/docs/api/java/io/InputStream.html#read--)