8245121: (bf) XBuffer.put(Xbuffer src) can give unexpected result when storage overlaps

[Paul Sandoz]

> On May 26, 2020, at 2:20 PM, Brian Burkhalter <brian.burkhalter at oracle.com> wrote:
> 
> 
>> On May 26, 2020, at 11:35 AM, Paul Sandoz <paul.sandoz at oracle.com <mailto:paul.sandoz at oracle.com>> wrote:
>> 
>> This is looking good,
>> 
>> Strictly speaking the fences are only required for the direct buffer, but I think I prefer them being placed where the unsafe usages are for tighter scope.
> 
> Actually since the specific direct and heap implementations are modified simply to invoke super.put($TypeBuffer) if the target buffer is read-write, then within the X-Buffer method we don’t really know whether the destination or source is direct without an explicit check. I suppose one could make use of the fences contingent on the value of isDirect().
> 

One approach is to wrap the super call for fences when direct, but I think there is no need.


>> I would be inclined to drop the Bits.JNI_COPY_TO_ARRAY_THRESHOLD, since it never applies to direct buffers. I bet the swapping. Maybe can be determined with follow evaluation after this issue.
> 
> You mean dropping the else branch and converting the else if branch to an else?
> 

Yes.


>> Test generally looks good:
>> 
>>   47     static final Random RND = new MyRandom(SEED); // XXX RandomFactory
>>   48 
>>   49     static class MyRandom extends Random {
>>   50         MyRandom(long seed) {
>>   51             super(seed);
>>   52         }
>> The additional methods are never used. No need to extend? (If there was a need the methods could use next accepting bits values).
> 
> It was done this way to make it simple to grab the appropriate next method based on the type:
>  163                 nextType = lookup.findVirtual(MyRandom.class,
>  164                      "next" + typeName, MethodType.methodType(elementType));
> Also, the method Random.next(int) is protected.
> 

Ah, sorry, I missed that, you can change the implementation to use next()? rather than rolling your own logic.

Paul.

>> I still hold out hope you can convert it to a testng test, with data providers and asserts, this is all good common practice for writing tests. The nested loops in test() effectively capture the test combinations input for two test cases. Let me know if you need some advice/help on this.
> 
> I’ll take another look.
> 
> Thanks,
> 
> Brian
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20200526/bfe56f78/attachment-0001.htm>