Re: [jdk16] RFR: 8259636: Check for buffer backed by shared segment kicks in in unexpected places [v2]

2021-01-12 Thread Maurizio Cimadamore
> When support for shared segment was added, we decided to disable support for 
> buffers derived from shared segment in certain async operations, as there's 
> currently no way to make sure that the memory won't be reclaimed while the IO 
> operation is still taking place.
> 
> After looking at the code, it seemed like the best place to put the 
> restriction would be sun.nio.ch.DirectBuffer::address() method, since this 
> method is used in a lot of places just before jumping into some piece of JNI 
> code.
> 
> While I still stand by that decision, the Netty team has discovered that this 
> decision also affected operations such as creating slices from byte buffers 
> derived from shared segment - this is caused by the fact that one direct 
> buffer constructor (the one for views and slices) is calling the dreaded 
> DirectBuffer::address method.
> 
> The fix is simple: just avoid the method call - which is very easy to do in 
> the case of the buffer constructor: in fact this method just returns the 
> value of the `address` field inside the `Buffer` class, so we can always cast 
> to `Buffer` and then access `address` field from there.

Maurizio Cimadamore has updated the pull request with a new target base due to 
a merge or a rebase. The pull request now contains two commits:

 - Merge branch 'master' into buffer_addr
 - Tweak direct buffer slice constructor not to use DirectBuffer::address

-

Changes: https://git.openjdk.java.net/jdk16/pull/110/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk16=110=01
  Stats: 11 lines in 2 files changed: 9 ins; 1 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk16/pull/110.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/110/head:pull/110

PR: https://git.openjdk.java.net/jdk16/pull/110


Re: [jdk16] RFR: 8259636: Check for buffer backed by shared segment kicks in in unexpected places

2021-01-12 Thread Chris Hegarty
On Tue, 12 Jan 2021 15:48:18 GMT, Maurizio Cimadamore  
wrote:

> When support for shared segment was added, we decided to disable support for 
> buffers derived from shared segment in certain async operations, as there's 
> currently no way to make sure that the memory won't be reclaimed while the IO 
> operation is still taking place.
> 
> After looking at the code, it seemed like the best place to put the 
> restriction would be sun.nio.ch.DirectBuffer::address() method, since this 
> method is used in a lot of places just before jumping into some piece of JNI 
> code.
> 
> While I still stand by that decision, the Netty team has discovered that this 
> decision also affected operations such as creating slices from byte buffers 
> derived from shared segment - this is caused by the fact that one direct 
> buffer constructor (the one for views and slices) is calling the dreaded 
> DirectBuffer::address method.
> 
> The fix is simple: just avoid the method call - which is very easy to do in 
> the case of the buffer constructor: in fact this method just returns the 
> value of the `address` field inside the `Buffer` class, so we can always cast 
> to `Buffer` and then access `address` field from there.

Marked as reviewed by chegar (Reviewer).

-

PR: https://git.openjdk.java.net/jdk16/pull/110


Re: [jdk16] RFR: 8259636: Check for buffer backed by shared segment kicks in in unexpected places

2021-01-12 Thread Alan Bateman
On Tue, 12 Jan 2021 15:48:18 GMT, Maurizio Cimadamore  
wrote:

> When support for shared segment was added, we decided to disable support for 
> buffers derived from shared segment in certain async operations, as there's 
> currently no way to make sure that the memory won't be reclaimed while the IO 
> operation is still taking place.
> 
> After looking at the code, it seemed like the best place to put the 
> restriction would be sun.nio.ch.DirectBuffer::address() method, since this 
> method is used in a lot of places just before jumping into some piece of JNI 
> code.
> 
> While I still stand by that decision, the Netty team has discovered that this 
> decision also affected operations such as creating slices from byte buffers 
> derived from shared segment - this is caused by the fact that one direct 
> buffer constructor (the one for views and slices) is calling the dreaded 
> DirectBuffer::address method.
> 
> The fix is simple: just avoid the method call - which is very easy to do in 
> the case of the buffer constructor: in fact this method just returns the 
> value of the `address` field inside the `Buffer` class, so we can always cast 
> to `Buffer` and then access `address` field from there.

Marked as reviewed by alanb (Reviewer).

-

PR: https://git.openjdk.java.net/jdk16/pull/110


Re: [jdk16] RFR: 8259636: Check for buffer backed by shared segment kicks in in unexpected places

2021-01-12 Thread Athijegannathan Sundararajan
On Tue, 12 Jan 2021 15:48:18 GMT, Maurizio Cimadamore  
wrote:

> When support for shared segment was added, we decided to disable support for 
> buffers derived from shared segment in certain async operations, as there's 
> currently no way to make sure that the memory won't be reclaimed while the IO 
> operation is still taking place.
> 
> After looking at the code, it seemed like the best place to put the 
> restriction would be sun.nio.ch.DirectBuffer::address() method, since this 
> method is used in a lot of places just before jumping into some piece of JNI 
> code.
> 
> While I still stand by that decision, the Netty team has discovered that this 
> decision also affected operations such as creating slices from byte buffers 
> derived from shared segment - this is caused by the fact that one direct 
> buffer constructor (the one for views and slices) is calling the dreaded 
> DirectBuffer::address method.
> 
> The fix is simple: just avoid the method call - which is very easy to do in 
> the case of the buffer constructor: in fact this method just returns the 
> value of the `address` field inside the `Buffer` class, so we can always cast 
> to `Buffer` and then access `address` field from there.

Marked as reviewed by sundar (Reviewer).

-

PR: https://git.openjdk.java.net/jdk16/pull/110


[jdk16] RFR: 8259636: Check for buffer backed by shared segment kicks in in unexpected places

2021-01-12 Thread Maurizio Cimadamore
When support for shared segment was added, we decided to disable support for 
buffers derived from shared segment in certain async operations, as there's 
currently no way to make sure that the memory won't be reclaimed while the IO 
operation is still taking place.

After looking at the code, it seemed like the best place to put the restriction 
would be sun.nio.ch.DirectBuffer::address() method, since this method is used 
in a lot of places just before jumping into some piece of JNI code.

While I still stand by that decision, the Netty team has discovered that this 
decision also affected operations such as creating slices from byte buffers 
derived from shared segment - this is caused by the fact that one direct buffer 
constructor (the one for views and slices) is calling the dreaded 
DirectBuffer::address method.

The fix is simple: just avoid the method call - which is very easy to do in the 
case of the buffer constructor: in fact this method just returns the value of 
the `address` field inside the `Buffer` class, so we can always cast to 
`Buffer` and then access `address` field from there.

-

Commit messages:
 - Tweak direct buffer slice constructor not to use DirectBuffer::address

Changes: https://git.openjdk.java.net/jdk16/pull/110/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk16=110=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8259636
  Stats: 19 lines in 2 files changed: 18 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk16/pull/110.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk16 pull/110/head:pull/110

PR: https://git.openjdk.java.net/jdk16/pull/110