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

Ekaterina Dimitrova commented on CASSANDRA-14752:
-------------------------------------------------

I reworked 3.11 and 4.0 and pushed another additional changes for trunk based 
on Stefania Alborghetti's patch. (I will add her as author at the end before 
commit)

4.0: Byte buffers of {{BooleanSerializer}} are now read-only. We cannot make 
them on-heap read-only, as we would need to change our code in several key 
places which seems not worth it at this point. However, buffers can be off-heap 
read-only. Note that this only offers partial protection against put calls done 
on the buffer itself. It will not protect, amongst several cases, for put calls 
where the read-only buffer is the source, as it was the case in 
{{AbstractCompositeType}}. In this case, the position will still be advanced. 
To make these buffers completely safe we would need to duplicate them and 
accept the additional GC pressure.
||Patch||CI||
|​[3.11|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:14752-3.11?expand=1]|[Jenkins|https://ci-cassandra.apache.org/job/Cassandra-devbranch/1171/#showFailuresLink]|
|​[4.0|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:14752-4.0?expand=1]|[Jenkins|https://ci-cassandra.apache.org/job/Cassandra-devbranch/1172/#showFailuresLink]|
|​[trunk|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:14752-trunk?expand=1]|[Jenkins|https://ci-cassandra.apache.org/job/Cassandra-devbranch/1183/#showFailuresLink]|

I don't see any related issues in the CI runs.

[~blerer], [~ifesdjeen], as you are already familiar with this, does anyone of 
you have time for review?

> serializers/BooleanSerializer.java is using static bytebuffers which may 
> cause problem for subsequent operations
> ----------------------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-14752
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-14752
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Legacy/Core
>            Reporter: Varun Barala
>            Assignee: Ekaterina Dimitrova
>            Priority: Normal
>             Fix For: 4.x
>
>         Attachments: patch, patch-modified
>
>
> [https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/serializers/BooleanSerializer.java#L26]
>  It has two static Bytebuffer variables:-
> {code:java}
> private static final ByteBuffer TRUE = ByteBuffer.wrap(new byte[]{1});
> private static final ByteBuffer FALSE = ByteBuffer.wrap(new byte[]{0});{code}
> What will happen if the position of these Bytebuffers is being changed by 
> some other operations? It'll affect other subsequent operations. -IMO Using 
> static is not a good idea here.-
> A potential place where it can become problematic: 
> [https://github.com/apache/cassandra/blob/cassandra-2.1.13/src/java/org/apache/cassandra/db/marshal/AbstractCompositeType.java#L243]
>  Since we are calling *`.remaining()`* It may give wrong results _i.e 0_ if 
> these Bytebuffers have been used previously.
> Solution: 
>  
> [https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/serializers/BooleanSerializer.java#L42]
>  Every time we return new bytebuffer object. Please do let me know If there 
> is a better way. I'd like to contribute. Thanks!!
> {code:java}
> public ByteBuffer serialize(Boolean value)
> {
> return (value == null) ? ByteBufferUtil.EMPTY_BYTE_BUFFER
> : value ? ByteBuffer.wrap(new byte[] {1}) : ByteBuffer.wrap(new byte[] {0}); 
> // false
> }
> {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to