[ 
https://issues.apache.org/jira/browse/HBASE-26197?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

chenglei updated HBASE-26197:
-----------------------------
    Description: 
MultiByteBuff.put(int destOffset, ByteBuff src, int srcOffset, int length) has 
some obvious bug:
* It is a common utility method and may be used in many situations, but its 
implementation seems mixup {{items}} in {{src}} {{MutiByteBuff}} and {{items}} 
in the  {{dest}} {{MultiByteBuff}} , just as line 749 and line  754 
illustrated.  The logical is right only when the  that src {{ByteBuff}} is also 
a {{MultiByteBuff}} and byte size of every {{ByteBuffer}} in {{src.items}} has 
exactly the same size as every {{ByteBuffer}} in the {{dest.items}},but looking 
the usage of this method in the hbase project, obviously the assumption is not 
right.
{code:java}:
746     public MultiByteBuff put(int offset, ByteBuff src, int srcOffset, int 
length) {
747         checkRefCount();
748         int destItemIndex = getItemIndex(offset);
749         int srcItemIndex = getItemIndex(srcOffset);
750         ByteBuffer destItem = this.items[destItemIndex];
751         offset = offset - this.itemBeginPos[destItemIndex];
752
753        ByteBuffer srcItem = getItemByteBuffer(src, srcItemIndex);
754        srcOffset = srcOffset - this.itemBeginPos[srcItemIndex];
            ...
{code}
       


* If src is {{SingleByteBuff}} and its remaining space is fewer than 
length,when remaining space is exhausted, this {{MultiByteBuff.put}} method 
would not throw any exception and continue to put src {{ByteBuff}} once again 
from position 0 because following {{MultiByteBuff.getItemByteBuffer}} ignores 
index paramter for  {{SingleByteBuff}} . Obviously, this behavior is much 
strange and unexpected.
  {code:java}
       private static ByteBuffer getItemByteBuffer(ByteBuff buf, int index) {
             return (buf instanceof SingleByteBuff) ? buf.nioByteBuffers()[0]
                   : ((MultiByteBuff) buf).items[index];
        }
   {code} 

Why seems test is OK with too much bugs? Because in normal cases, we just use 
{{SingleByteBuff}} not {{MultiByteBuff}}.

  was:
MultiByteBuff.put(int destOffset, ByteBuff src, int srcOffset, int length) has 
some obvious bug:
* It is a common utility method and may be used in many situations, but its 
implementation seems  assuming that src {{ByteBuff}} is also a 
{{MultiByteBuff}} and byte size of every {{ByteBuffer}} in {{src.items}} has 
exactly the same size as every {{ByteBuffer}}  in the dest 
{{MultiByteBuff}},just as line 749 and line  754 illustrated:
{code:java}:
746     public MultiByteBuff put(int offset, ByteBuff src, int srcOffset, int 
length) {
747         checkRefCount();
748         int destItemIndex = getItemIndex(offset);
749         int srcItemIndex = getItemIndex(srcOffset);
750         ByteBuffer destItem = this.items[destItemIndex];
751         offset = offset - this.itemBeginPos[destItemIndex];
752
753        ByteBuffer srcItem = getItemByteBuffer(src, srcItemIndex);
754        srcOffset = srcOffset - this.itemBeginPos[srcItemIndex];
            ...
{code}
       But looking the usage of this method in the hbase project, obviously the 
assumption is not right and even in following 
{{MultiByteBuff.getItemByteBuffer}} which inside the above 
{{MultiByteBuff.put}} method, it also considing the case the src {{ByteBuff}} 
may be {{SingleByteBuff}}, which is in contradiction with line 754 in the above 
{{MultiByteBuff.put}}:

*    {code:java}
       private static ByteBuffer getItemByteBuffer(ByteBuff buf, int index) {
             return (buf instanceof SingleByteBuff) ? buf.nioByteBuffers()[0]
                   : ((MultiByteBuff) buf).items[index];
        }
   {code} 

* If src is {{SingleByteBuff}} and its remaining space is fewer than 
length,when remaining space is exhausted, this {{MultiByteBuff.put}} method 
would not throw any exception and continue to put src {{ByteBuff}} again from 
position 0 because above {{MultiByteBuff.getItemByteBuffer}} ignores index 
paramter for   {{SingleByteBuff}} . Obviously, this behavior is much strange 
and unexpected.



> Fix some obvious bugs in MultiByteBuff.put
> ------------------------------------------
>
>                 Key: HBASE-26197
>                 URL: https://issues.apache.org/jira/browse/HBASE-26197
>             Project: HBase
>          Issue Type: Bug
>    Affects Versions: 3.0.0-alpha-1, 2.4.5
>            Reporter: chenglei
>            Priority: Major
>
> MultiByteBuff.put(int destOffset, ByteBuff src, int srcOffset, int length) 
> has some obvious bug:
> * It is a common utility method and may be used in many situations, but its 
> implementation seems mixup {{items}} in {{src}} {{MutiByteBuff}} and 
> {{items}} in the  {{dest}} {{MultiByteBuff}} , just as line 749 and line  754 
> illustrated.  The logical is right only when the  that src {{ByteBuff}} is 
> also a {{MultiByteBuff}} and byte size of every {{ByteBuffer}} in 
> {{src.items}} has exactly the same size as every {{ByteBuffer}} in the 
> {{dest.items}},but looking the usage of this method in the hbase project, 
> obviously the assumption is not right.
> {code:java}:
> 746     public MultiByteBuff put(int offset, ByteBuff src, int srcOffset, int 
> length) {
> 747         checkRefCount();
> 748         int destItemIndex = getItemIndex(offset);
> 749         int srcItemIndex = getItemIndex(srcOffset);
> 750         ByteBuffer destItem = this.items[destItemIndex];
> 751         offset = offset - this.itemBeginPos[destItemIndex];
> 752
> 753        ByteBuffer srcItem = getItemByteBuffer(src, srcItemIndex);
> 754        srcOffset = srcOffset - this.itemBeginPos[srcItemIndex];
>             ...
> {code}
>        
> * If src is {{SingleByteBuff}} and its remaining space is fewer than 
> length,when remaining space is exhausted, this {{MultiByteBuff.put}} method 
> would not throw any exception and continue to put src {{ByteBuff}} once again 
> from position 0 because following {{MultiByteBuff.getItemByteBuffer}} ignores 
> index paramter for  {{SingleByteBuff}} . Obviously, this behavior is much 
> strange and unexpected.
>   {code:java}
>        private static ByteBuffer getItemByteBuffer(ByteBuff buf, int index) {
>              return (buf instanceof SingleByteBuff) ? buf.nioByteBuffers()[0]
>                    : ((MultiByteBuff) buf).items[index];
>         }
>    {code} 
> Why seems test is OK with too much bugs? Because in normal cases, we just use 
> {{SingleByteBuff}} not {{MultiByteBuff}}.



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

Reply via email to