cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ariel Weisberg (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-9241) ByteBuffer.array() without ByteBuffer.arrayOffset() + ByteBuffer.position() is a bug
Date Wed, 12 Aug 2015 15:55:45 GMT

    [ https://issues.apache.org/jira/browse/CASSANDRA-9241?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14693716#comment-14693716
] 

Ariel Weisberg commented on CASSANDRA-9241:
-------------------------------------------

Calling buffer.hasArray() isn't necessary because ByteBuffer is already doing to throw if
you call array() when there is none.

One of the pitfalls of ByteBuffers is thread safety. If multiple threads use the relative
get()/set() methods they race to modify the position. The work around is to duplicate buffers
before using APIs that modify the position and limit which is error prone and creates garbage.
So it is useful to know that something preserves the position and limit and does not manipulate
it even temporarily. It also indicates that implementers of the API shouldn't introduce an
implementation that modifies the position and limit.

The CI builds failed again. I cherry-picked your nits and rebased to latest trunk. Will try
again.

> ByteBuffer.array() without ByteBuffer.arrayOffset() + ByteBuffer.position() is a bug
> ------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-9241
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-9241
>             Project: Cassandra
>          Issue Type: Bug
>            Reporter: Ariel Weisberg
>            Assignee: Ariel Weisberg
>            Priority: Minor
>             Fix For: 3.0.x
>
>
> I found one instance of this on OHCProvider so it make sense to review all usages since
there aren't that many.
> Some suspect things:
> https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/utils/FastByteOperations.java#L197
> https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/db/ColumnFamilyStore.java#L1877
> https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/gms/TokenSerializer.java#L40
> https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/io/compress/CompressedRandomAccessReader.java#L178
> https://github.com/apache/cassandra/blob/trunk/tools/stress/src/org/apache/cassandra/stress/operations/predefined/CqlOperation.java#L104
> https://github.com/apache/cassandra/blob/trunk/tools/stress/src/org/apache/cassandra/stress/operations/predefined/CqlOperation.java#L543
> https://github.com/apache/cassandra/blob/trunk/tools/stress/src/org/apache/cassandra/stress/operations/predefined/CqlOperation.java#L563
> I made this list off of 8099 so I might have missed some instances on trunk. FastByteOperations
makes me cross eyed so it is worth a second pass to make sure offsets in byte buffers are
handled correctly.
> Generally I like to use the full incantation even when I have done things like allocate
the buffer on the stack locally for copy pasta/refactoring reasons and to make clear to new
users how the API is supposed to work.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message