DirectByteBuffers and reachabilityFence

classic Classic list List threaded Threaded
148 messages Options
1234 ... 8
Reply | Threaded
Open this post in threaded view
|

DirectByteBuffers and reachabilityFence

Alexandre De Champeaux
Hi all,

I recently had a look at the discussion started by Peter Levart on October 21 (http://cs.oswego.edu/pipermail/concurrency-interest/2015-October/014493.html).

It was a very insightful discussion, and made me aware that the "this" object could be garbage collected while being inside a call of one of its method.

However, this got me concerned about the java.nio.DirectByteBuffer read and write methods:
If the "this" object is garbage collected when making a call like ByteBuffer.allocateDirect(int).someGetOrPutMethod(), the native pointer that is passed to sun.misc.Unsafe might be freed, and accessing it will cause the read/write to occur in an invalid memory area, which might lead to a segfault, or other major issues.
This would be quite unlikely: JIT compilation needs to occur while keeping a safepoint, then a GC must happen, and finally the ReferenceHandler thread needs to perform its cleanup fast enough.

I am particularly concerned by the get(byte[] dst, int offset, int length) method, that in turns calls Bits.copyToArray, which purposely splits its calls to Unsafe.copyMemory to allow for safepoints to sneak in.

Am I correct, or does the JVM performs specific protection for instances of DirectByteBuffer?

Regards,  

Alexandre de Champeaux

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Andrew Haley
On 12/07/2015 12:40 PM, Alexandre De Champeaux wrote:

> However, this got me concerned about the java.nio.DirectByteBuffer read and
> write methods:
> If the "this" object is garbage collected when making a call like
> ByteBuffer.allocateDirect(int).someGetOrPutMethod(), the native pointer
> that is passed to sun.misc.Unsafe might be freed, and accessing it will
> cause the read/write to occur in an invalid memory area, which might lead
> to a segfault, or other major issues.
> This would be quite unlikely: JIT compilation needs to occur while keeping
> a safepoint, then a GC must happen, and finally the ReferenceHandler thread
> needs to perform its cleanup fast enough.
>
> I am particularly concerned by the get(byte[] dst, int offset, int length)
> method, that in turns calls Bits.copyToArray, which purposely splits its
> calls to Unsafe.copyMemory to allow for safepoints to sneak in.
>
> Am I correct, or does the JVM performs specific protection for instances of
> DirectByteBuffer?

There are many places where we need to insert a reachabilityFence.
This is one such.  Some non-HotSpot JVMs perform their cleanups
very aggressively, so it's more likely there.

Andrew.

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Vitaly Davidovich
In reply to this post by Alexandre De Champeaux

JIT knows about Unsafe operations, and it also knows the type of memory being accessed (or sometimes knows it doesn't know :)).  So I don't think it'll mark a DBB as unreachable while these operations are in-flight.

Peter's scenario is unique to WeakReference since it's intentionally not considered a strong reference and there's otherwise plain java code in his example (that JIT can reason about easily otherwise).

sent from my phone

On Dec 7, 2015 8:10 AM, "Alexandre De Champeaux" <[hidden email]> wrote:
Hi all,

I recently had a look at the discussion started by Peter Levart on October 21 (http://cs.oswego.edu/pipermail/concurrency-interest/2015-October/014493.html).

It was a very insightful discussion, and made me aware that the "this" object could be garbage collected while being inside a call of one of its method.

However, this got me concerned about the java.nio.DirectByteBuffer read and write methods:
If the "this" object is garbage collected when making a call like ByteBuffer.allocateDirect(int).someGetOrPutMethod(), the native pointer that is passed to sun.misc.Unsafe might be freed, and accessing it will cause the read/write to occur in an invalid memory area, which might lead to a segfault, or other major issues.
This would be quite unlikely: JIT compilation needs to occur while keeping a safepoint, then a GC must happen, and finally the ReferenceHandler thread needs to perform its cleanup fast enough.

I am particularly concerned by the get(byte[] dst, int offset, int length) method, that in turns calls Bits.copyToArray, which purposely splits its calls to Unsafe.copyMemory to allow for safepoints to sneak in.

Am I correct, or does the JVM performs specific protection for instances of DirectByteBuffer?

Regards,  

Alexandre de Champeaux

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Vitaly Davidovich
In reply to this post by Andrew Haley

If reachabilityFence use is going to proliferate, especially in perf sensitive places, Hotspot will need to make this method simply a liveness marker and not emit a call like the current prototype/version is doing.

sent from my phone

On Dec 7, 2015 10:07 AM, "Andrew Haley" <[hidden email]> wrote:
On 12/07/2015 12:40 PM, Alexandre De Champeaux wrote:
> However, this got me concerned about the java.nio.DirectByteBuffer read and
> write methods:
> If the "this" object is garbage collected when making a call like
> ByteBuffer.allocateDirect(int).someGetOrPutMethod(), the native pointer
> that is passed to sun.misc.Unsafe might be freed, and accessing it will
> cause the read/write to occur in an invalid memory area, which might lead
> to a segfault, or other major issues.
> This would be quite unlikely: JIT compilation needs to occur while keeping
> a safepoint, then a GC must happen, and finally the ReferenceHandler thread
> needs to perform its cleanup fast enough.
>
> I am particularly concerned by the get(byte[] dst, int offset, int length)
> method, that in turns calls Bits.copyToArray, which purposely splits its
> calls to Unsafe.copyMemory to allow for safepoints to sneak in.
>
> Am I correct, or does the JVM performs specific protection for instances of
> DirectByteBuffer?

There are many places where we need to insert a reachabilityFence.
This is one such.  Some non-HotSpot JVMs perform their cleanups
very aggressively, so it's more likely there.

Andrew.

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Alexandre De Champeaux
In reply to this post by Vitaly Davidovich
I have actually written a test class that reproduces a segfault. It uses an structure similar to DBB and an updated version of the Bits.copyToArray method which forces a GC and waits a bit from within it to simulate a safepoint stop and a GC, and leaves time to the reference handler to clean up.
Is this test too far away from the DBB code to actually simulate a possible behavior of DBB? Or is hotspot only able to not mark as unreachable vanilla java DBB, but not customer classes?

Here is the test class:


import sun.misc.Cleaner;
import sun.misc.Unsafe;

/**
 * This test checks whether or not the JVM will dereference "this" and destroy the buffer while
 * accessing it.
 * <p>
 * Run the main method of this class to run the test.
 * <p>
 * Note that the test segfaults each time if using a JNI call to mmap/munmap to do the memory
 * management but not as often using Unsafe (malloc recycling memory?).
 * <p>
 * Example output of a failure:
 *
 * <pre>
TestGcThisAndDestroyBuffer.main() 0 -- 0
TestGcThisAndDestroyBuffer.main() 0 -- 1
TestGcThisAndDestroyBuffer.main() 0 -- 2
TestGcThisAndDestroyBuffer.main() 0 -- 3
TestGcThisAndDestroyBuffer.main() 0 -- 4
TestGcThisAndDestroyBuffer.main() 0 -- 5
TestGcThisAndDestroyBuffer.main() 0 -- 6
TestGcThisAndDestroyBuffer.main() 0 -- 7
TestGcThisAndDestroyBuffer.main() 0 -- 8
TestGcThisAndDestroyBuffer.main() 0 -- 9
TestGcThisAndDestroyBuffer.main() 0 -- 10
TestGcThisAndDestroyBuffer.main() 0 -- 11
TestGcThisAndDestroyBuffer.main() 0 -- 12
TestGcThisAndDestroyBuffer.main() 0 -- 13
TestGcThisAndDestroyBuffer.main() 0 -- 14
Must be compiled now, start doing some GCs 30001
#
# A fatal error has been detected by the Java Runtime Environment:
#
#  SIGSEGV (0xb) at pc=0x00007f489fb86220, pid=16949, tid=139949914818304
#
# JRE version: Java(TM) SE Runtime Environment (8.0_60-b27) (build 1.8.0_60-b27)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (25.60-b23 mixed mode linux-amd64 compressed oops)
# Problematic frame:
# V  [libjvm.so+0x7f7220]
#
# Failed to write core dump. Core dumps have been disabled. To enable core dumping, try "ulimit -c unlimited" before starting Java again
#
# An error report file with more information is saved as:
# /SSDhome/alexandre/ActivePivot5/scripts/adc/java_tests/hs_err_pid16949.log
Compiled method (nm)   10180   18     n 0       sun.misc.Unsafe::copyMemory (native)
total in heap  [0x00007f488910f2d0,0x00007f488910f640] = 880
relocation     [0x00007f488910f3f8,0x00007f488910f440] = 72
main code      [0x00007f488910f440,0x00007f488910f640] = 512
Compiled method (nm)   10180   18     n 0       sun.misc.Unsafe::copyMemory (native)
total in heap  [0x00007f488910f2d0,0x00007f488910f640] = 880
relocation     [0x00007f488910f3f8,0x00007f488910f440] = 72
main code      [0x00007f488910f440,0x00007f488910f640] = 512
#
# If you would like to submit a bug report, please visit:
#
 * </pre>
 */
@SuppressWarnings("restriction")
public class TestGcThisAndDestroyBuffer {

/** The threshold found in Bits.copyMemory */
static final long UNSAFE_COPY_THRESHOLD = 1024L * 1024L;
private static final Unsafe unsafe = retrieveUnsafe();

static long ctr = 0;

// The tested method
// See the method in Bits.copyToArray
/* For Bits.copyToArray to segfault we need:
*
* 1) The buffer to be garbage collected while calling the method, for that we need:
* - A safepoint exist in the method
* - The buffer needs to not be held, so JIT compil must have occured,
* and JIT must have dereferenced the DBB at the time of the call
* 2) The reference handler thread to have run the associated cleaner
*/
static void copyToArray(long srcAddr, Object dst, long dstBaseOffset, long dstPos, long length) {
long offset = dstBaseOffset + dstPos;
while (length > 0) {
// Code differing from Bits.
if (ctr++ >= 30_000) {
// Counter is there to wait for JIT compilation
System.out.println("Must be compiled now, start doing some GCs " + ctr);
// Here we simulate a safepoint, and a GC at this safepoint
System.gc();
try {
// And here we wait for the reference handler to perform the cleaning to simulate a fast cleaning.
Thread.sleep(1000);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
}

long size = (length > UNSAFE_COPY_THRESHOLD) ? UNSAFE_COPY_THRESHOLD : length;
unsafe.copyMemory(null, srcAddr, dst, offset, size);
length -= size;
srcAddr += size;
offset += size;
}
}

// A basic class inspired from java.nio.DirectByteBuffer
static class SimpleBuffer {
static int SIZE = (int) (2 * UNSAFE_COPY_THRESHOLD);
static final long arrayBaseOffset = (long) unsafe.arrayBaseOffset(byte[].class);

final long addr;

public SimpleBuffer() {
addr = unsafe.allocateMemory(SIZE);
unsafe.setMemory(addr, SIZE, (byte) 0);
Cleaner.create(this, new Deallocator(addr));
}

public byte[] copy() {
final byte[] a = new byte[SIZE];
copyToArray(addr, a, arrayBaseOffset, 0, SIZE);
return a;
}

private static class Deallocator implements Runnable {

private long address;

private Deallocator(long address) {
assert (address != 0);
this.address = address;
}

@Override
public void run() {
if (address == 0) {
// Paranoia
return;
}
unsafe.freeMemory(address);
address = 0;
}

}

}

static byte[] aBunchOfCopies() {
byte[] res = null;
for (int i = 0; i < 1000; ++i) {
res = new SimpleBuffer().copy();
if (res[0] == -1) {
return res;
}
}
return res;
}

public static void main(String[] args) {
int i = 0;
while (true) {
final byte[] res = aBunchOfCopies();
System.out.println("TestGcThisAndDestroyBuffer.main() " + res[0] + " -- " + i++);
}
}

private static final sun.misc.Unsafe retrieveUnsafe() {
try {
return sun.misc.Unsafe.getUnsafe();
} catch (SecurityException se) {
try {
return java.security.AccessController
.doPrivileged(new java.security.PrivilegedExceptionAction<sun.misc.Unsafe>() {
@Override
public sun.misc.Unsafe run() throws Exception {
java.lang.reflect.Field f = sun.misc.Unsafe.class.getDeclaredField("theUnsafe");
f.setAccessible(true);
return (sun.misc.Unsafe) f.get(null);
}
});
} catch (java.security.PrivilegedActionException e) {
throw new RuntimeException("Could not initialize intrinsics", e.getCause());
}
}
}

}


On Mon, Dec 7, 2015 at 4:15 PM, Vitaly Davidovich <[hidden email]> wrote:

JIT knows about Unsafe operations, and it also knows the type of memory being accessed (or sometimes knows it doesn't know :)).  So I don't think it'll mark a DBB as unreachable while these operations are in-flight.

Peter's scenario is unique to WeakReference since it's intentionally not considered a strong reference and there's otherwise plain java code in his example (that JIT can reason about easily otherwise).

sent from my phone

On Dec 7, 2015 8:10 AM, "Alexandre De Champeaux" <[hidden email]> wrote:
Hi all,

I recently had a look at the discussion started by Peter Levart on October 21 (http://cs.oswego.edu/pipermail/concurrency-interest/2015-October/014493.html).

It was a very insightful discussion, and made me aware that the "this" object could be garbage collected while being inside a call of one of its method.

However, this got me concerned about the java.nio.DirectByteBuffer read and write methods:
If the "this" object is garbage collected when making a call like ByteBuffer.allocateDirect(int).someGetOrPutMethod(), the native pointer that is passed to sun.misc.Unsafe might be freed, and accessing it will cause the read/write to occur in an invalid memory area, which might lead to a segfault, or other major issues.
This would be quite unlikely: JIT compilation needs to occur while keeping a safepoint, then a GC must happen, and finally the ReferenceHandler thread needs to perform its cleanup fast enough.

I am particularly concerned by the get(byte[] dst, int offset, int length) method, that in turns calls Bits.copyToArray, which purposely splits its calls to Unsafe.copyMemory to allow for safepoints to sneak in.

Am I correct, or does the JVM performs specific protection for instances of DirectByteBuffer?

Regards,  

Alexandre de Champeaux

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest




--
Alexandre

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Vitaly Davidovich
Can you more closely mirror DBB by updating some state of your SimpleBuffer after calling copyToArray()? DBB returns 'this' from its get(byte[], ...) and also sets position after the Bits.copyToArray() method.

On Mon, Dec 7, 2015 at 12:08 PM, Alexandre De Champeaux <[hidden email]> wrote:
I have actually written a test class that reproduces a segfault. It uses an structure similar to DBB and an updated version of the Bits.copyToArray method which forces a GC and waits a bit from within it to simulate a safepoint stop and a GC, and leaves time to the reference handler to clean up.
Is this test too far away from the DBB code to actually simulate a possible behavior of DBB? Or is hotspot only able to not mark as unreachable vanilla java DBB, but not customer classes?

Here is the test class:


import sun.misc.Cleaner;
import sun.misc.Unsafe;

/**
 * This test checks whether or not the JVM will dereference "this" and destroy the buffer while
 * accessing it.
 * <p>
 * Run the main method of this class to run the test.
 * <p>
 * Note that the test segfaults each time if using a JNI call to mmap/munmap to do the memory
 * management but not as often using Unsafe (malloc recycling memory?).
 * <p>
 * Example output of a failure:
 *
 * <pre>
TestGcThisAndDestroyBuffer.main() 0 -- 0
TestGcThisAndDestroyBuffer.main() 0 -- 1
TestGcThisAndDestroyBuffer.main() 0 -- 2
TestGcThisAndDestroyBuffer.main() 0 -- 3
TestGcThisAndDestroyBuffer.main() 0 -- 4
TestGcThisAndDestroyBuffer.main() 0 -- 5
TestGcThisAndDestroyBuffer.main() 0 -- 6
TestGcThisAndDestroyBuffer.main() 0 -- 7
TestGcThisAndDestroyBuffer.main() 0 -- 8
TestGcThisAndDestroyBuffer.main() 0 -- 9
TestGcThisAndDestroyBuffer.main() 0 -- 10
TestGcThisAndDestroyBuffer.main() 0 -- 11
TestGcThisAndDestroyBuffer.main() 0 -- 12
TestGcThisAndDestroyBuffer.main() 0 -- 13
TestGcThisAndDestroyBuffer.main() 0 -- 14
Must be compiled now, start doing some GCs 30001
#
# A fatal error has been detected by the Java Runtime Environment:
#
#  SIGSEGV (0xb) at pc=0x00007f489fb86220, pid=16949, tid=139949914818304
#
# JRE version: Java(TM) SE Runtime Environment (8.0_60-b27) (build 1.8.0_60-b27)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (25.60-b23 mixed mode linux-amd64 compressed oops)
# Problematic frame:
# V  [libjvm.so+0x7f7220]
#
# Failed to write core dump. Core dumps have been disabled. To enable core dumping, try "ulimit -c unlimited" before starting Java again
#
# An error report file with more information is saved as:
# /SSDhome/alexandre/ActivePivot5/scripts/adc/java_tests/hs_err_pid16949.log
Compiled method (nm)   10180   18     n 0       sun.misc.Unsafe::copyMemory (native)
total in heap  [0x00007f488910f2d0,0x00007f488910f640] = 880
relocation     [0x00007f488910f3f8,0x00007f488910f440] = 72
main code      [0x00007f488910f440,0x00007f488910f640] = 512
Compiled method (nm)   10180   18     n 0       sun.misc.Unsafe::copyMemory (native)
total in heap  [0x00007f488910f2d0,0x00007f488910f640] = 880
relocation     [0x00007f488910f3f8,0x00007f488910f440] = 72
main code      [0x00007f488910f440,0x00007f488910f640] = 512
#
# If you would like to submit a bug report, please visit:
#
 * </pre>
 */
@SuppressWarnings("restriction")
public class TestGcThisAndDestroyBuffer {

/** The threshold found in Bits.copyMemory */
static final long UNSAFE_COPY_THRESHOLD = 1024L * 1024L;
private static final Unsafe unsafe = retrieveUnsafe();

static long ctr = 0;

// The tested method
// See the method in Bits.copyToArray
/* For Bits.copyToArray to segfault we need:
*
* 1) The buffer to be garbage collected while calling the method, for that we need:
* - A safepoint exist in the method
* - The buffer needs to not be held, so JIT compil must have occured,
* and JIT must have dereferenced the DBB at the time of the call
* 2) The reference handler thread to have run the associated cleaner
*/
static void copyToArray(long srcAddr, Object dst, long dstBaseOffset, long dstPos, long length) {
long offset = dstBaseOffset + dstPos;
while (length > 0) {
// Code differing from Bits.
if (ctr++ >= 30_000) {
// Counter is there to wait for JIT compilation
System.out.println("Must be compiled now, start doing some GCs " + ctr);
// Here we simulate a safepoint, and a GC at this safepoint
System.gc();
try {
// And here we wait for the reference handler to perform the cleaning to simulate a fast cleaning.
Thread.sleep(1000);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
}

long size = (length > UNSAFE_COPY_THRESHOLD) ? UNSAFE_COPY_THRESHOLD : length;
unsafe.copyMemory(null, srcAddr, dst, offset, size);
length -= size;
srcAddr += size;
offset += size;
}
}

// A basic class inspired from java.nio.DirectByteBuffer
static class SimpleBuffer {
static int SIZE = (int) (2 * UNSAFE_COPY_THRESHOLD);
static final long arrayBaseOffset = (long) unsafe.arrayBaseOffset(byte[].class);

final long addr;

public SimpleBuffer() {
addr = unsafe.allocateMemory(SIZE);
unsafe.setMemory(addr, SIZE, (byte) 0);
Cleaner.create(this, new Deallocator(addr));
}

public byte[] copy() {
final byte[] a = new byte[SIZE];
copyToArray(addr, a, arrayBaseOffset, 0, SIZE);
return a;
}

private static class Deallocator implements Runnable {

private long address;

private Deallocator(long address) {
assert (address != 0);
this.address = address;
}

@Override
public void run() {
if (address == 0) {
// Paranoia
return;
}
unsafe.freeMemory(address);
address = 0;
}

}

}

static byte[] aBunchOfCopies() {
byte[] res = null;
for (int i = 0; i < 1000; ++i) {
res = new SimpleBuffer().copy();
if (res[0] == -1) {
return res;
}
}
return res;
}

public static void main(String[] args) {
int i = 0;
while (true) {
final byte[] res = aBunchOfCopies();
System.out.println("TestGcThisAndDestroyBuffer.main() " + res[0] + " -- " + i++);
}
}

private static final sun.misc.Unsafe retrieveUnsafe() {
try {
return sun.misc.Unsafe.getUnsafe();
} catch (SecurityException se) {
try {
return java.security.AccessController
.doPrivileged(new java.security.PrivilegedExceptionAction<sun.misc.Unsafe>() {
@Override
public sun.misc.Unsafe run() throws Exception {
java.lang.reflect.Field f = sun.misc.Unsafe.class.getDeclaredField("theUnsafe");
f.setAccessible(true);
return (sun.misc.Unsafe) f.get(null);
}
});
} catch (java.security.PrivilegedActionException e) {
throw new RuntimeException("Could not initialize intrinsics", e.getCause());
}
}
}

}


On Mon, Dec 7, 2015 at 4:15 PM, Vitaly Davidovich <[hidden email]> wrote:

JIT knows about Unsafe operations, and it also knows the type of memory being accessed (or sometimes knows it doesn't know :)).  So I don't think it'll mark a DBB as unreachable while these operations are in-flight.

Peter's scenario is unique to WeakReference since it's intentionally not considered a strong reference and there's otherwise plain java code in his example (that JIT can reason about easily otherwise).

sent from my phone

On Dec 7, 2015 8:10 AM, "Alexandre De Champeaux" <[hidden email]> wrote:
Hi all,

I recently had a look at the discussion started by Peter Levart on October 21 (http://cs.oswego.edu/pipermail/concurrency-interest/2015-October/014493.html).

It was a very insightful discussion, and made me aware that the "this" object could be garbage collected while being inside a call of one of its method.

However, this got me concerned about the java.nio.DirectByteBuffer read and write methods:
If the "this" object is garbage collected when making a call like ByteBuffer.allocateDirect(int).someGetOrPutMethod(), the native pointer that is passed to sun.misc.Unsafe might be freed, and accessing it will cause the read/write to occur in an invalid memory area, which might lead to a segfault, or other major issues.
This would be quite unlikely: JIT compilation needs to occur while keeping a safepoint, then a GC must happen, and finally the ReferenceHandler thread needs to perform its cleanup fast enough.

I am particularly concerned by the get(byte[] dst, int offset, int length) method, that in turns calls Bits.copyToArray, which purposely splits its calls to Unsafe.copyMemory to allow for safepoints to sneak in.

Am I correct, or does the JVM performs specific protection for instances of DirectByteBuffer?

Regards,  

Alexandre de Champeaux

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest




--
Alexandre


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Alexandre De Champeaux
Yep things are fine if I add a call to a method similar to the position() method of DBB after calling copyToArray(). 

On Mon, Dec 7, 2015 at 6:23 PM, Vitaly Davidovich <[hidden email]> wrote:
Can you more closely mirror DBB by updating some state of your SimpleBuffer after calling copyToArray()? DBB returns 'this' from its get(byte[], ...) and also sets position after the Bits.copyToArray() method.

On Mon, Dec 7, 2015 at 12:08 PM, Alexandre De Champeaux <[hidden email]> wrote:
I have actually written a test class that reproduces a segfault. It uses an structure similar to DBB and an updated version of the Bits.copyToArray method which forces a GC and waits a bit from within it to simulate a safepoint stop and a GC, and leaves time to the reference handler to clean up.
Is this test too far away from the DBB code to actually simulate a possible behavior of DBB? Or is hotspot only able to not mark as unreachable vanilla java DBB, but not customer classes?

Here is the test class:


import sun.misc.Cleaner;
import sun.misc.Unsafe;

/**
 * This test checks whether or not the JVM will dereference "this" and destroy the buffer while
 * accessing it.
 * <p>
 * Run the main method of this class to run the test.
 * <p>
 * Note that the test segfaults each time if using a JNI call to mmap/munmap to do the memory
 * management but not as often using Unsafe (malloc recycling memory?).
 * <p>
 * Example output of a failure:
 *
 * <pre>
TestGcThisAndDestroyBuffer.main() 0 -- 0
TestGcThisAndDestroyBuffer.main() 0 -- 1
TestGcThisAndDestroyBuffer.main() 0 -- 2
TestGcThisAndDestroyBuffer.main() 0 -- 3
TestGcThisAndDestroyBuffer.main() 0 -- 4
TestGcThisAndDestroyBuffer.main() 0 -- 5
TestGcThisAndDestroyBuffer.main() 0 -- 6
TestGcThisAndDestroyBuffer.main() 0 -- 7
TestGcThisAndDestroyBuffer.main() 0 -- 8
TestGcThisAndDestroyBuffer.main() 0 -- 9
TestGcThisAndDestroyBuffer.main() 0 -- 10
TestGcThisAndDestroyBuffer.main() 0 -- 11
TestGcThisAndDestroyBuffer.main() 0 -- 12
TestGcThisAndDestroyBuffer.main() 0 -- 13
TestGcThisAndDestroyBuffer.main() 0 -- 14
Must be compiled now, start doing some GCs 30001
#
# A fatal error has been detected by the Java Runtime Environment:
#
#  SIGSEGV (0xb) at pc=0x00007f489fb86220, pid=16949, tid=139949914818304
#
# JRE version: Java(TM) SE Runtime Environment (8.0_60-b27) (build 1.8.0_60-b27)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (25.60-b23 mixed mode linux-amd64 compressed oops)
# Problematic frame:
# V  [libjvm.so+0x7f7220]
#
# Failed to write core dump. Core dumps have been disabled. To enable core dumping, try "ulimit -c unlimited" before starting Java again
#
# An error report file with more information is saved as:
# /SSDhome/alexandre/ActivePivot5/scripts/adc/java_tests/hs_err_pid16949.log
Compiled method (nm)   10180   18     n 0       sun.misc.Unsafe::copyMemory (native)
total in heap  [0x00007f488910f2d0,0x00007f488910f640] = 880
relocation     [0x00007f488910f3f8,0x00007f488910f440] = 72
main code      [0x00007f488910f440,0x00007f488910f640] = 512
Compiled method (nm)   10180   18     n 0       sun.misc.Unsafe::copyMemory (native)
total in heap  [0x00007f488910f2d0,0x00007f488910f640] = 880
relocation     [0x00007f488910f3f8,0x00007f488910f440] = 72
main code      [0x00007f488910f440,0x00007f488910f640] = 512
#
# If you would like to submit a bug report, please visit:
#
 * </pre>
 */
@SuppressWarnings("restriction")
public class TestGcThisAndDestroyBuffer {

/** The threshold found in Bits.copyMemory */
static final long UNSAFE_COPY_THRESHOLD = 1024L * 1024L;
private static final Unsafe unsafe = retrieveUnsafe();

static long ctr = 0;

// The tested method
// See the method in Bits.copyToArray
/* For Bits.copyToArray to segfault we need:
*
* 1) The buffer to be garbage collected while calling the method, for that we need:
* - A safepoint exist in the method
* - The buffer needs to not be held, so JIT compil must have occured,
* and JIT must have dereferenced the DBB at the time of the call
* 2) The reference handler thread to have run the associated cleaner
*/
static void copyToArray(long srcAddr, Object dst, long dstBaseOffset, long dstPos, long length) {
long offset = dstBaseOffset + dstPos;
while (length > 0) {
// Code differing from Bits.
if (ctr++ >= 30_000) {
// Counter is there to wait for JIT compilation
System.out.println("Must be compiled now, start doing some GCs " + ctr);
// Here we simulate a safepoint, and a GC at this safepoint
System.gc();
try {
// And here we wait for the reference handler to perform the cleaning to simulate a fast cleaning.
Thread.sleep(1000);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
}

long size = (length > UNSAFE_COPY_THRESHOLD) ? UNSAFE_COPY_THRESHOLD : length;
unsafe.copyMemory(null, srcAddr, dst, offset, size);
length -= size;
srcAddr += size;
offset += size;
}
}

// A basic class inspired from java.nio.DirectByteBuffer
static class SimpleBuffer {
static int SIZE = (int) (2 * UNSAFE_COPY_THRESHOLD);
static final long arrayBaseOffset = (long) unsafe.arrayBaseOffset(byte[].class);

final long addr;

public SimpleBuffer() {
addr = unsafe.allocateMemory(SIZE);
unsafe.setMemory(addr, SIZE, (byte) 0);
Cleaner.create(this, new Deallocator(addr));
}

public byte[] copy() {
final byte[] a = new byte[SIZE];
copyToArray(addr, a, arrayBaseOffset, 0, SIZE);
return a;
}

private static class Deallocator implements Runnable {

private long address;

private Deallocator(long address) {
assert (address != 0);
this.address = address;
}

@Override
public void run() {
if (address == 0) {
// Paranoia
return;
}
unsafe.freeMemory(address);
address = 0;
}

}

}

static byte[] aBunchOfCopies() {
byte[] res = null;
for (int i = 0; i < 1000; ++i) {
res = new SimpleBuffer().copy();
if (res[0] == -1) {
return res;
}
}
return res;
}

public static void main(String[] args) {
int i = 0;
while (true) {
final byte[] res = aBunchOfCopies();
System.out.println("TestGcThisAndDestroyBuffer.main() " + res[0] + " -- " + i++);
}
}

private static final sun.misc.Unsafe retrieveUnsafe() {
try {
return sun.misc.Unsafe.getUnsafe();
} catch (SecurityException se) {
try {
return java.security.AccessController
.doPrivileged(new java.security.PrivilegedExceptionAction<sun.misc.Unsafe>() {
@Override
public sun.misc.Unsafe run() throws Exception {
java.lang.reflect.Field f = sun.misc.Unsafe.class.getDeclaredField("theUnsafe");
f.setAccessible(true);
return (sun.misc.Unsafe) f.get(null);
}
});
} catch (java.security.PrivilegedActionException e) {
throw new RuntimeException("Could not initialize intrinsics", e.getCause());
}
}
}

}


On Mon, Dec 7, 2015 at 4:15 PM, Vitaly Davidovich <[hidden email]> wrote:

JIT knows about Unsafe operations, and it also knows the type of memory being accessed (or sometimes knows it doesn't know :)).  So I don't think it'll mark a DBB as unreachable while these operations are in-flight.

Peter's scenario is unique to WeakReference since it's intentionally not considered a strong reference and there's otherwise plain java code in his example (that JIT can reason about easily otherwise).

sent from my phone

On Dec 7, 2015 8:10 AM, "Alexandre De Champeaux" <[hidden email]> wrote:
Hi all,

I recently had a look at the discussion started by Peter Levart on October 21 (http://cs.oswego.edu/pipermail/concurrency-interest/2015-October/014493.html).

It was a very insightful discussion, and made me aware that the "this" object could be garbage collected while being inside a call of one of its method.

However, this got me concerned about the java.nio.DirectByteBuffer read and write methods:
If the "this" object is garbage collected when making a call like ByteBuffer.allocateDirect(int).someGetOrPutMethod(), the native pointer that is passed to sun.misc.Unsafe might be freed, and accessing it will cause the read/write to occur in an invalid memory area, which might lead to a segfault, or other major issues.
This would be quite unlikely: JIT compilation needs to occur while keeping a safepoint, then a GC must happen, and finally the ReferenceHandler thread needs to perform its cleanup fast enough.

I am particularly concerned by the get(byte[] dst, int offset, int length) method, that in turns calls Bits.copyToArray, which purposely splits its calls to Unsafe.copyMemory to allow for safepoints to sneak in.

Am I correct, or does the JVM performs specific protection for instances of DirectByteBuffer?

Regards,  

Alexandre de Champeaux

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest




--
Alexandre




--
Alexandre

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Vitaly Davidovich
Ok, good :).

So the difference between your original case and what DBB has is that the "natural" lifetime of your SimpleBuffer ends prior to the unsafe operation.  Once you "detach" a piece of unmanaged memory from a managed wrapper (e.g. take the long address and start passing it around without the wrapper), all bets are off.  But while the managed wrapper is "naturally" live (i.e. as interpreter would treat it, or at least without subsequent compiler optimizations) I'd expect (hope!) that the wrapper does not become unreachable while those ops are in flight.

On Mon, Dec 7, 2015 at 12:46 PM, Alexandre De Champeaux <[hidden email]> wrote:
Yep things are fine if I add a call to a method similar to the position() method of DBB after calling copyToArray(). 

On Mon, Dec 7, 2015 at 6:23 PM, Vitaly Davidovich <[hidden email]> wrote:
Can you more closely mirror DBB by updating some state of your SimpleBuffer after calling copyToArray()? DBB returns 'this' from its get(byte[], ...) and also sets position after the Bits.copyToArray() method.

On Mon, Dec 7, 2015 at 12:08 PM, Alexandre De Champeaux <[hidden email]> wrote:
I have actually written a test class that reproduces a segfault. It uses an structure similar to DBB and an updated version of the Bits.copyToArray method which forces a GC and waits a bit from within it to simulate a safepoint stop and a GC, and leaves time to the reference handler to clean up.
Is this test too far away from the DBB code to actually simulate a possible behavior of DBB? Or is hotspot only able to not mark as unreachable vanilla java DBB, but not customer classes?

Here is the test class:


import sun.misc.Cleaner;
import sun.misc.Unsafe;

/**
 * This test checks whether or not the JVM will dereference "this" and destroy the buffer while
 * accessing it.
 * <p>
 * Run the main method of this class to run the test.
 * <p>
 * Note that the test segfaults each time if using a JNI call to mmap/munmap to do the memory
 * management but not as often using Unsafe (malloc recycling memory?).
 * <p>
 * Example output of a failure:
 *
 * <pre>
TestGcThisAndDestroyBuffer.main() 0 -- 0
TestGcThisAndDestroyBuffer.main() 0 -- 1
TestGcThisAndDestroyBuffer.main() 0 -- 2
TestGcThisAndDestroyBuffer.main() 0 -- 3
TestGcThisAndDestroyBuffer.main() 0 -- 4
TestGcThisAndDestroyBuffer.main() 0 -- 5
TestGcThisAndDestroyBuffer.main() 0 -- 6
TestGcThisAndDestroyBuffer.main() 0 -- 7
TestGcThisAndDestroyBuffer.main() 0 -- 8
TestGcThisAndDestroyBuffer.main() 0 -- 9
TestGcThisAndDestroyBuffer.main() 0 -- 10
TestGcThisAndDestroyBuffer.main() 0 -- 11
TestGcThisAndDestroyBuffer.main() 0 -- 12
TestGcThisAndDestroyBuffer.main() 0 -- 13
TestGcThisAndDestroyBuffer.main() 0 -- 14
Must be compiled now, start doing some GCs 30001
#
# A fatal error has been detected by the Java Runtime Environment:
#
#  SIGSEGV (0xb) at pc=0x00007f489fb86220, pid=16949, tid=139949914818304
#
# JRE version: Java(TM) SE Runtime Environment (8.0_60-b27) (build 1.8.0_60-b27)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (25.60-b23 mixed mode linux-amd64 compressed oops)
# Problematic frame:
# V  [libjvm.so+0x7f7220]
#
# Failed to write core dump. Core dumps have been disabled. To enable core dumping, try "ulimit -c unlimited" before starting Java again
#
# An error report file with more information is saved as:
# /SSDhome/alexandre/ActivePivot5/scripts/adc/java_tests/hs_err_pid16949.log
Compiled method (nm)   10180   18     n 0       sun.misc.Unsafe::copyMemory (native)
total in heap  [0x00007f488910f2d0,0x00007f488910f640] = 880
relocation     [0x00007f488910f3f8,0x00007f488910f440] = 72
main code      [0x00007f488910f440,0x00007f488910f640] = 512
Compiled method (nm)   10180   18     n 0       sun.misc.Unsafe::copyMemory (native)
total in heap  [0x00007f488910f2d0,0x00007f488910f640] = 880
relocation     [0x00007f488910f3f8,0x00007f488910f440] = 72
main code      [0x00007f488910f440,0x00007f488910f640] = 512
#
# If you would like to submit a bug report, please visit:
#
 * </pre>
 */
@SuppressWarnings("restriction")
public class TestGcThisAndDestroyBuffer {

/** The threshold found in Bits.copyMemory */
static final long UNSAFE_COPY_THRESHOLD = 1024L * 1024L;
private static final Unsafe unsafe = retrieveUnsafe();

static long ctr = 0;

// The tested method
// See the method in Bits.copyToArray
/* For Bits.copyToArray to segfault we need:
*
* 1) The buffer to be garbage collected while calling the method, for that we need:
* - A safepoint exist in the method
* - The buffer needs to not be held, so JIT compil must have occured,
* and JIT must have dereferenced the DBB at the time of the call
* 2) The reference handler thread to have run the associated cleaner
*/
static void copyToArray(long srcAddr, Object dst, long dstBaseOffset, long dstPos, long length) {
long offset = dstBaseOffset + dstPos;
while (length > 0) {
// Code differing from Bits.
if (ctr++ >= 30_000) {
// Counter is there to wait for JIT compilation
System.out.println("Must be compiled now, start doing some GCs " + ctr);
// Here we simulate a safepoint, and a GC at this safepoint
System.gc();
try {
// And here we wait for the reference handler to perform the cleaning to simulate a fast cleaning.
Thread.sleep(1000);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
}

long size = (length > UNSAFE_COPY_THRESHOLD) ? UNSAFE_COPY_THRESHOLD : length;
unsafe.copyMemory(null, srcAddr, dst, offset, size);
length -= size;
srcAddr += size;
offset += size;
}
}

// A basic class inspired from java.nio.DirectByteBuffer
static class SimpleBuffer {
static int SIZE = (int) (2 * UNSAFE_COPY_THRESHOLD);
static final long arrayBaseOffset = (long) unsafe.arrayBaseOffset(byte[].class);

final long addr;

public SimpleBuffer() {
addr = unsafe.allocateMemory(SIZE);
unsafe.setMemory(addr, SIZE, (byte) 0);
Cleaner.create(this, new Deallocator(addr));
}

public byte[] copy() {
final byte[] a = new byte[SIZE];
copyToArray(addr, a, arrayBaseOffset, 0, SIZE);
return a;
}

private static class Deallocator implements Runnable {

private long address;

private Deallocator(long address) {
assert (address != 0);
this.address = address;
}

@Override
public void run() {
if (address == 0) {
// Paranoia
return;
}
unsafe.freeMemory(address);
address = 0;
}

}

}

static byte[] aBunchOfCopies() {
byte[] res = null;
for (int i = 0; i < 1000; ++i) {
res = new SimpleBuffer().copy();
if (res[0] == -1) {
return res;
}
}
return res;
}

public static void main(String[] args) {
int i = 0;
while (true) {
final byte[] res = aBunchOfCopies();
System.out.println("TestGcThisAndDestroyBuffer.main() " + res[0] + " -- " + i++);
}
}

private static final sun.misc.Unsafe retrieveUnsafe() {
try {
return sun.misc.Unsafe.getUnsafe();
} catch (SecurityException se) {
try {
return java.security.AccessController
.doPrivileged(new java.security.PrivilegedExceptionAction<sun.misc.Unsafe>() {
@Override
public sun.misc.Unsafe run() throws Exception {
java.lang.reflect.Field f = sun.misc.Unsafe.class.getDeclaredField("theUnsafe");
f.setAccessible(true);
return (sun.misc.Unsafe) f.get(null);
}
});
} catch (java.security.PrivilegedActionException e) {
throw new RuntimeException("Could not initialize intrinsics", e.getCause());
}
}
}

}


On Mon, Dec 7, 2015 at 4:15 PM, Vitaly Davidovich <[hidden email]> wrote:

JIT knows about Unsafe operations, and it also knows the type of memory being accessed (or sometimes knows it doesn't know :)).  So I don't think it'll mark a DBB as unreachable while these operations are in-flight.

Peter's scenario is unique to WeakReference since it's intentionally not considered a strong reference and there's otherwise plain java code in his example (that JIT can reason about easily otherwise).

sent from my phone

On Dec 7, 2015 8:10 AM, "Alexandre De Champeaux" <[hidden email]> wrote:
Hi all,

I recently had a look at the discussion started by Peter Levart on October 21 (http://cs.oswego.edu/pipermail/concurrency-interest/2015-October/014493.html).

It was a very insightful discussion, and made me aware that the "this" object could be garbage collected while being inside a call of one of its method.

However, this got me concerned about the java.nio.DirectByteBuffer read and write methods:
If the "this" object is garbage collected when making a call like ByteBuffer.allocateDirect(int).someGetOrPutMethod(), the native pointer that is passed to sun.misc.Unsafe might be freed, and accessing it will cause the read/write to occur in an invalid memory area, which might lead to a segfault, or other major issues.
This would be quite unlikely: JIT compilation needs to occur while keeping a safepoint, then a GC must happen, and finally the ReferenceHandler thread needs to perform its cleanup fast enough.

I am particularly concerned by the get(byte[] dst, int offset, int length) method, that in turns calls Bits.copyToArray, which purposely splits its calls to Unsafe.copyMemory to allow for safepoints to sneak in.

Am I correct, or does the JVM performs specific protection for instances of DirectByteBuffer?

Regards,  

Alexandre de Champeaux

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest




--
Alexandre




--
Alexandre


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Alexandre De Champeaux
Ok thanks.
Correct me if I'm wrong, but basically what you are saying is that hotspot does not perform advanced enough analysis to see that 'this' is actually unnecessary after the call to unsafe. So there is no practical risk to read invalid data/crash, but only a theoretical one.
What about the standard get() methods of DBB? Since they do not update the state of the buffer after the call, are they also protected in some way at the practical level? Or is it unlikely (impossible?) to have a GC safepoint just at the end of the ix(int) method? I guess even if this happen, you would need to be very very unlucky to have the reference handler clean the Cleaners fast enough...


On Mon, Dec 7, 2015 at 7:03 PM, Vitaly Davidovich <[hidden email]> wrote:
Ok, good :).

So the difference between your original case and what DBB has is that the "natural" lifetime of your SimpleBuffer ends prior to the unsafe operation.  Once you "detach" a piece of unmanaged memory from a managed wrapper (e.g. take the long address and start passing it around without the wrapper), all bets are off.  But while the managed wrapper is "naturally" live (i.e. as interpreter would treat it, or at least without subsequent compiler optimizations) I'd expect (hope!) that the wrapper does not become unreachable while those ops are in flight.

On Mon, Dec 7, 2015 at 12:46 PM, Alexandre De Champeaux <[hidden email]> wrote:
Yep things are fine if I add a call to a method similar to the position() method of DBB after calling copyToArray(). 

On Mon, Dec 7, 2015 at 6:23 PM, Vitaly Davidovich <[hidden email]> wrote:
Can you more closely mirror DBB by updating some state of your SimpleBuffer after calling copyToArray()? DBB returns 'this' from its get(byte[], ...) and also sets position after the Bits.copyToArray() method.

On Mon, Dec 7, 2015 at 12:08 PM, Alexandre De Champeaux <[hidden email]> wrote:
I have actually written a test class that reproduces a segfault. It uses an structure similar to DBB and an updated version of the Bits.copyToArray method which forces a GC and waits a bit from within it to simulate a safepoint stop and a GC, and leaves time to the reference handler to clean up.
Is this test too far away from the DBB code to actually simulate a possible behavior of DBB? Or is hotspot only able to not mark as unreachable vanilla java DBB, but not customer classes?

Here is the test class:


import sun.misc.Cleaner;
import sun.misc.Unsafe;

/**
 * This test checks whether or not the JVM will dereference "this" and destroy the buffer while
 * accessing it.
 * <p>
 * Run the main method of this class to run the test.
 * <p>
 * Note that the test segfaults each time if using a JNI call to mmap/munmap to do the memory
 * management but not as often using Unsafe (malloc recycling memory?).
 * <p>
 * Example output of a failure:
 *
 * <pre>
TestGcThisAndDestroyBuffer.main() 0 -- 0
TestGcThisAndDestroyBuffer.main() 0 -- 1
TestGcThisAndDestroyBuffer.main() 0 -- 2
TestGcThisAndDestroyBuffer.main() 0 -- 3
TestGcThisAndDestroyBuffer.main() 0 -- 4
TestGcThisAndDestroyBuffer.main() 0 -- 5
TestGcThisAndDestroyBuffer.main() 0 -- 6
TestGcThisAndDestroyBuffer.main() 0 -- 7
TestGcThisAndDestroyBuffer.main() 0 -- 8
TestGcThisAndDestroyBuffer.main() 0 -- 9
TestGcThisAndDestroyBuffer.main() 0 -- 10
TestGcThisAndDestroyBuffer.main() 0 -- 11
TestGcThisAndDestroyBuffer.main() 0 -- 12
TestGcThisAndDestroyBuffer.main() 0 -- 13
TestGcThisAndDestroyBuffer.main() 0 -- 14
Must be compiled now, start doing some GCs 30001
#
# A fatal error has been detected by the Java Runtime Environment:
#
#  SIGSEGV (0xb) at pc=0x00007f489fb86220, pid=16949, tid=139949914818304
#
# JRE version: Java(TM) SE Runtime Environment (8.0_60-b27) (build 1.8.0_60-b27)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (25.60-b23 mixed mode linux-amd64 compressed oops)
# Problematic frame:
# V  [libjvm.so+0x7f7220]
#
# Failed to write core dump. Core dumps have been disabled. To enable core dumping, try "ulimit -c unlimited" before starting Java again
#
# An error report file with more information is saved as:
# /SSDhome/alexandre/ActivePivot5/scripts/adc/java_tests/hs_err_pid16949.log
Compiled method (nm)   10180   18     n 0       sun.misc.Unsafe::copyMemory (native)
total in heap  [0x00007f488910f2d0,0x00007f488910f640] = 880
relocation     [0x00007f488910f3f8,0x00007f488910f440] = 72
main code      [0x00007f488910f440,0x00007f488910f640] = 512
Compiled method (nm)   10180   18     n 0       sun.misc.Unsafe::copyMemory (native)
total in heap  [0x00007f488910f2d0,0x00007f488910f640] = 880
relocation     [0x00007f488910f3f8,0x00007f488910f440] = 72
main code      [0x00007f488910f440,0x00007f488910f640] = 512
#
# If you would like to submit a bug report, please visit:
#
 * </pre>
 */
@SuppressWarnings("restriction")
public class TestGcThisAndDestroyBuffer {

/** The threshold found in Bits.copyMemory */
static final long UNSAFE_COPY_THRESHOLD = 1024L * 1024L;
private static final Unsafe unsafe = retrieveUnsafe();

static long ctr = 0;

// The tested method
// See the method in Bits.copyToArray
/* For Bits.copyToArray to segfault we need:
*
* 1) The buffer to be garbage collected while calling the method, for that we need:
* - A safepoint exist in the method
* - The buffer needs to not be held, so JIT compil must have occured,
* and JIT must have dereferenced the DBB at the time of the call
* 2) The reference handler thread to have run the associated cleaner
*/
static void copyToArray(long srcAddr, Object dst, long dstBaseOffset, long dstPos, long length) {
long offset = dstBaseOffset + dstPos;
while (length > 0) {
// Code differing from Bits.
if (ctr++ >= 30_000) {
// Counter is there to wait for JIT compilation
System.out.println("Must be compiled now, start doing some GCs " + ctr);
// Here we simulate a safepoint, and a GC at this safepoint
System.gc();
try {
// And here we wait for the reference handler to perform the cleaning to simulate a fast cleaning.
Thread.sleep(1000);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
}

long size = (length > UNSAFE_COPY_THRESHOLD) ? UNSAFE_COPY_THRESHOLD : length;
unsafe.copyMemory(null, srcAddr, dst, offset, size);
length -= size;
srcAddr += size;
offset += size;
}
}

// A basic class inspired from java.nio.DirectByteBuffer
static class SimpleBuffer {
static int SIZE = (int) (2 * UNSAFE_COPY_THRESHOLD);
static final long arrayBaseOffset = (long) unsafe.arrayBaseOffset(byte[].class);

final long addr;

public SimpleBuffer() {
addr = unsafe.allocateMemory(SIZE);
unsafe.setMemory(addr, SIZE, (byte) 0);
Cleaner.create(this, new Deallocator(addr));
}

public byte[] copy() {
final byte[] a = new byte[SIZE];
copyToArray(addr, a, arrayBaseOffset, 0, SIZE);
return a;
}

private static class Deallocator implements Runnable {

private long address;

private Deallocator(long address) {
assert (address != 0);
this.address = address;
}

@Override
public void run() {
if (address == 0) {
// Paranoia
return;
}
unsafe.freeMemory(address);
address = 0;
}

}

}

static byte[] aBunchOfCopies() {
byte[] res = null;
for (int i = 0; i < 1000; ++i) {
res = new SimpleBuffer().copy();
if (res[0] == -1) {
return res;
}
}
return res;
}

public static void main(String[] args) {
int i = 0;
while (true) {
final byte[] res = aBunchOfCopies();
System.out.println("TestGcThisAndDestroyBuffer.main() " + res[0] + " -- " + i++);
}
}

private static final sun.misc.Unsafe retrieveUnsafe() {
try {
return sun.misc.Unsafe.getUnsafe();
} catch (SecurityException se) {
try {
return java.security.AccessController
.doPrivileged(new java.security.PrivilegedExceptionAction<sun.misc.Unsafe>() {
@Override
public sun.misc.Unsafe run() throws Exception {
java.lang.reflect.Field f = sun.misc.Unsafe.class.getDeclaredField("theUnsafe");
f.setAccessible(true);
return (sun.misc.Unsafe) f.get(null);
}
});
} catch (java.security.PrivilegedActionException e) {
throw new RuntimeException("Could not initialize intrinsics", e.getCause());
}
}
}

}


On Mon, Dec 7, 2015 at 4:15 PM, Vitaly Davidovich <[hidden email]> wrote:

JIT knows about Unsafe operations, and it also knows the type of memory being accessed (or sometimes knows it doesn't know :)).  So I don't think it'll mark a DBB as unreachable while these operations are in-flight.

Peter's scenario is unique to WeakReference since it's intentionally not considered a strong reference and there's otherwise plain java code in his example (that JIT can reason about easily otherwise).

sent from my phone

On Dec 7, 2015 8:10 AM, "Alexandre De Champeaux" <[hidden email]> wrote:
Hi all,

I recently had a look at the discussion started by Peter Levart on October 21 (http://cs.oswego.edu/pipermail/concurrency-interest/2015-October/014493.html).

It was a very insightful discussion, and made me aware that the "this" object could be garbage collected while being inside a call of one of its method.

However, this got me concerned about the java.nio.DirectByteBuffer read and write methods:
If the "this" object is garbage collected when making a call like ByteBuffer.allocateDirect(int).someGetOrPutMethod(), the native pointer that is passed to sun.misc.Unsafe might be freed, and accessing it will cause the read/write to occur in an invalid memory area, which might lead to a segfault, or other major issues.
This would be quite unlikely: JIT compilation needs to occur while keeping a safepoint, then a GC must happen, and finally the ReferenceHandler thread needs to perform its cleanup fast enough.

I am particularly concerned by the get(byte[] dst, int offset, int length) method, that in turns calls Bits.copyToArray, which purposely splits its calls to Unsafe.copyMemory to allow for safepoints to sneak in.

Am I correct, or does the JVM performs specific protection for instances of DirectByteBuffer?

Regards,  

Alexandre de Champeaux

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest




--
Alexandre




--
Alexandre




--
Alexandre

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Vitaly Davidovich
Correct me if I'm wrong, but basically what you are saying is that hotspot does not perform advanced enough analysis to see that 'this' is actually unnecessary after the call to unsafe.

In which case? In DBB, `this` is still necessary after the Unsafe call since it needs to update position.  In theory, it could move position update prior to calling Bits.copyToArray() but it would have to prove that it's safe to do so.  Part of that safety check would require it to reason about memory effects, side effects, and failure cases (i.e. position update isn't reached if there's an exception, it would need to preserve that effect, also taking into account that it may not be able to speculatively write to memory and then undo those effects in case of exception).  So the answer is probably "yes" to your question above, in a theoretical sense only.

What about the standard get() methods of DBB? Since they do not update the state of the buffer after the call, are they also protected in some way at the practical level? Or is it unlikely (impossible?) to have a GC safepoint just at the end of the ix(int) method? I guess even if this happen, you would need to be very very unlucky to have the reference handler clean the Cleaners fast enough...

I think the reasoning here is similar to the above.  ByteBuffer.get() is invoked on a DBB instance, compiler sees a call to Unsafe.getXXX at some point.  The "natural" lifetime of the instance survives until the instance method returns.  So if compiler now wanted to shorten this by observing that `this` is unreachable sooner than method exit, it has to prove a bunch of things like I mentioned above.

Mind you, this is just my understanding -- I'm not speaking to this in any authoritative manner :).

On Mon, Dec 7, 2015 at 1:36 PM, Alexandre De Champeaux <[hidden email]> wrote:
Ok thanks.
Correct me if I'm wrong, but basically what you are saying is that hotspot does not perform advanced enough analysis to see that 'this' is actually unnecessary after the call to unsafe. So there is no practical risk to read invalid data/crash, but only a theoretical one.
What about the standard get() methods of DBB? Since they do not update the state of the buffer after the call, are they also protected in some way at the practical level? Or is it unlikely (impossible?) to have a GC safepoint just at the end of the ix(int) method? I guess even if this happen, you would need to be very very unlucky to have the reference handler clean the Cleaners fast enough...


On Mon, Dec 7, 2015 at 7:03 PM, Vitaly Davidovich <[hidden email]> wrote:
Ok, good :).

So the difference between your original case and what DBB has is that the "natural" lifetime of your SimpleBuffer ends prior to the unsafe operation.  Once you "detach" a piece of unmanaged memory from a managed wrapper (e.g. take the long address and start passing it around without the wrapper), all bets are off.  But while the managed wrapper is "naturally" live (i.e. as interpreter would treat it, or at least without subsequent compiler optimizations) I'd expect (hope!) that the wrapper does not become unreachable while those ops are in flight.

On Mon, Dec 7, 2015 at 12:46 PM, Alexandre De Champeaux <[hidden email]> wrote:
Yep things are fine if I add a call to a method similar to the position() method of DBB after calling copyToArray(). 

On Mon, Dec 7, 2015 at 6:23 PM, Vitaly Davidovich <[hidden email]> wrote:
Can you more closely mirror DBB by updating some state of your SimpleBuffer after calling copyToArray()? DBB returns 'this' from its get(byte[], ...) and also sets position after the Bits.copyToArray() method.

On Mon, Dec 7, 2015 at 12:08 PM, Alexandre De Champeaux <[hidden email]> wrote:
I have actually written a test class that reproduces a segfault. It uses an structure similar to DBB and an updated version of the Bits.copyToArray method which forces a GC and waits a bit from within it to simulate a safepoint stop and a GC, and leaves time to the reference handler to clean up.
Is this test too far away from the DBB code to actually simulate a possible behavior of DBB? Or is hotspot only able to not mark as unreachable vanilla java DBB, but not customer classes?

Here is the test class:


import sun.misc.Cleaner;
import sun.misc.Unsafe;

/**
 * This test checks whether or not the JVM will dereference "this" and destroy the buffer while
 * accessing it.
 * <p>
 * Run the main method of this class to run the test.
 * <p>
 * Note that the test segfaults each time if using a JNI call to mmap/munmap to do the memory
 * management but not as often using Unsafe (malloc recycling memory?).
 * <p>
 * Example output of a failure:
 *
 * <pre>
TestGcThisAndDestroyBuffer.main() 0 -- 0
TestGcThisAndDestroyBuffer.main() 0 -- 1
TestGcThisAndDestroyBuffer.main() 0 -- 2
TestGcThisAndDestroyBuffer.main() 0 -- 3
TestGcThisAndDestroyBuffer.main() 0 -- 4
TestGcThisAndDestroyBuffer.main() 0 -- 5
TestGcThisAndDestroyBuffer.main() 0 -- 6
TestGcThisAndDestroyBuffer.main() 0 -- 7
TestGcThisAndDestroyBuffer.main() 0 -- 8
TestGcThisAndDestroyBuffer.main() 0 -- 9
TestGcThisAndDestroyBuffer.main() 0 -- 10
TestGcThisAndDestroyBuffer.main() 0 -- 11
TestGcThisAndDestroyBuffer.main() 0 -- 12
TestGcThisAndDestroyBuffer.main() 0 -- 13
TestGcThisAndDestroyBuffer.main() 0 -- 14
Must be compiled now, start doing some GCs 30001
#
# A fatal error has been detected by the Java Runtime Environment:
#
#  SIGSEGV (0xb) at pc=0x00007f489fb86220, pid=16949, tid=139949914818304
#
# JRE version: Java(TM) SE Runtime Environment (8.0_60-b27) (build 1.8.0_60-b27)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (25.60-b23 mixed mode linux-amd64 compressed oops)
# Problematic frame:
# V  [libjvm.so+0x7f7220]
#
# Failed to write core dump. Core dumps have been disabled. To enable core dumping, try "ulimit -c unlimited" before starting Java again
#
# An error report file with more information is saved as:
# /SSDhome/alexandre/ActivePivot5/scripts/adc/java_tests/hs_err_pid16949.log
Compiled method (nm)   10180   18     n 0       sun.misc.Unsafe::copyMemory (native)
total in heap  [0x00007f488910f2d0,0x00007f488910f640] = 880
relocation     [0x00007f488910f3f8,0x00007f488910f440] = 72
main code      [0x00007f488910f440,0x00007f488910f640] = 512
Compiled method (nm)   10180   18     n 0       sun.misc.Unsafe::copyMemory (native)
total in heap  [0x00007f488910f2d0,0x00007f488910f640] = 880
relocation     [0x00007f488910f3f8,0x00007f488910f440] = 72
main code      [0x00007f488910f440,0x00007f488910f640] = 512
#
# If you would like to submit a bug report, please visit:
#
 * </pre>
 */
@SuppressWarnings("restriction")
public class TestGcThisAndDestroyBuffer {

/** The threshold found in Bits.copyMemory */
static final long UNSAFE_COPY_THRESHOLD = 1024L * 1024L;
private static final Unsafe unsafe = retrieveUnsafe();

static long ctr = 0;

// The tested method
// See the method in Bits.copyToArray
/* For Bits.copyToArray to segfault we need:
*
* 1) The buffer to be garbage collected while calling the method, for that we need:
* - A safepoint exist in the method
* - The buffer needs to not be held, so JIT compil must have occured,
* and JIT must have dereferenced the DBB at the time of the call
* 2) The reference handler thread to have run the associated cleaner
*/
static void copyToArray(long srcAddr, Object dst, long dstBaseOffset, long dstPos, long length) {
long offset = dstBaseOffset + dstPos;
while (length > 0) {
// Code differing from Bits.
if (ctr++ >= 30_000) {
// Counter is there to wait for JIT compilation
System.out.println("Must be compiled now, start doing some GCs " + ctr);
// Here we simulate a safepoint, and a GC at this safepoint
System.gc();
try {
// And here we wait for the reference handler to perform the cleaning to simulate a fast cleaning.
Thread.sleep(1000);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
}

long size = (length > UNSAFE_COPY_THRESHOLD) ? UNSAFE_COPY_THRESHOLD : length;
unsafe.copyMemory(null, srcAddr, dst, offset, size);
length -= size;
srcAddr += size;
offset += size;
}
}

// A basic class inspired from java.nio.DirectByteBuffer
static class SimpleBuffer {
static int SIZE = (int) (2 * UNSAFE_COPY_THRESHOLD);
static final long arrayBaseOffset = (long) unsafe.arrayBaseOffset(byte[].class);

final long addr;

public SimpleBuffer() {
addr = unsafe.allocateMemory(SIZE);
unsafe.setMemory(addr, SIZE, (byte) 0);
Cleaner.create(this, new Deallocator(addr));
}

public byte[] copy() {
final byte[] a = new byte[SIZE];
copyToArray(addr, a, arrayBaseOffset, 0, SIZE);
return a;
}

private static class Deallocator implements Runnable {

private long address;

private Deallocator(long address) {
assert (address != 0);
this.address = address;
}

@Override
public void run() {
if (address == 0) {
// Paranoia
return;
}
unsafe.freeMemory(address);
address = 0;
}

}

}

static byte[] aBunchOfCopies() {
byte[] res = null;
for (int i = 0; i < 1000; ++i) {
res = new SimpleBuffer().copy();
if (res[0] == -1) {
return res;
}
}
return res;
}

public static void main(String[] args) {
int i = 0;
while (true) {
final byte[] res = aBunchOfCopies();
System.out.println("TestGcThisAndDestroyBuffer.main() " + res[0] + " -- " + i++);
}
}

private static final sun.misc.Unsafe retrieveUnsafe() {
try {
return sun.misc.Unsafe.getUnsafe();
} catch (SecurityException se) {
try {
return java.security.AccessController
.doPrivileged(new java.security.PrivilegedExceptionAction<sun.misc.Unsafe>() {
@Override
public sun.misc.Unsafe run() throws Exception {
java.lang.reflect.Field f = sun.misc.Unsafe.class.getDeclaredField("theUnsafe");
f.setAccessible(true);
return (sun.misc.Unsafe) f.get(null);
}
});
} catch (java.security.PrivilegedActionException e) {
throw new RuntimeException("Could not initialize intrinsics", e.getCause());
}
}
}

}


On Mon, Dec 7, 2015 at 4:15 PM, Vitaly Davidovich <[hidden email]> wrote:

JIT knows about Unsafe operations, and it also knows the type of memory being accessed (or sometimes knows it doesn't know :)).  So I don't think it'll mark a DBB as unreachable while these operations are in-flight.

Peter's scenario is unique to WeakReference since it's intentionally not considered a strong reference and there's otherwise plain java code in his example (that JIT can reason about easily otherwise).

sent from my phone

On Dec 7, 2015 8:10 AM, "Alexandre De Champeaux" <[hidden email]> wrote:
Hi all,

I recently had a look at the discussion started by Peter Levart on October 21 (http://cs.oswego.edu/pipermail/concurrency-interest/2015-October/014493.html).

It was a very insightful discussion, and made me aware that the "this" object could be garbage collected while being inside a call of one of its method.

However, this got me concerned about the java.nio.DirectByteBuffer read and write methods:
If the "this" object is garbage collected when making a call like ByteBuffer.allocateDirect(int).someGetOrPutMethod(), the native pointer that is passed to sun.misc.Unsafe might be freed, and accessing it will cause the read/write to occur in an invalid memory area, which might lead to a segfault, or other major issues.
This would be quite unlikely: JIT compilation needs to occur while keeping a safepoint, then a GC must happen, and finally the ReferenceHandler thread needs to perform its cleanup fast enough.

I am particularly concerned by the get(byte[] dst, int offset, int length) method, that in turns calls Bits.copyToArray, which purposely splits its calls to Unsafe.copyMemory to allow for safepoints to sneak in.

Am I correct, or does the JVM performs specific protection for instances of DirectByteBuffer?

Regards,  

Alexandre de Champeaux

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest




--
Alexandre




--
Alexandre




--
Alexandre


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Andrew Haley
On 07/12/15 19:17, Vitaly Davidovich wrote:

>>
>> Correct me if I'm wrong, but basically what you are saying is that hotspot
>> does not perform advanced enough analysis to see that 'this' is actually
>> unnecessary after the call to unsafe.
>
>
> In which case? In DBB, `this` is still necessary after the Unsafe call
> since it needs to update position.  In theory, it could move position
> update prior to calling Bits.copyToArray() but it would have to prove that
> it's safe to do so.

That's not at all hard for HotSpot to do.

> Part of that safety check would require it to reason about memory
> effects, side effects, and failure cases (i.e. position update isn't
> reached if there's an exception,

What exception?  HotSpot knows everything about Bits.copyToArray()
because it is inlined.  It knows about all the arguments and
eliminates bounds checks.

> it would need to preserve that effect, also taking into account that
> it may not be able to speculatively write to memory and then undo
> those effects in case of exception).  So the answer is probably
> "yes" to your question above, in a theoretical sense only.

Not at all.

> I think the reasoning here is similar to the above.
> ByteBuffer.get() is invoked on a DBB instance, compiler sees a call
> to Unsafe.getXXX at some point.  The "natural" lifetime of the
> instance survives until the instance method returns.

No, this is completely wrong.

> So if compiler now wanted to shorten this by observing that `this`
> is unreachable sooner than method exit, it has to prove a bunch of
> things like I mentioned above.

Now inline everything.

Please stop sepeculating about what an optimizer can and can't do.
Please instead concerntrate on the semantics of the language and think
about correctness.

Andrew.
_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Andrew Haley
In reply to this post by Vitaly Davidovich
On 07/12/15 15:20, Vitaly Davidovich wrote:
> If reachabilityFence use is going to proliferate, especially in perf
> sensitive places, Hotspot will need to make this method simply a
> liveness marker and not emit a call like the current
> prototype/version is doing.

Perhaps, but we've already discussed that, and it's not so hard.
Surely correctness comes first, then we optimize.  I expect we're
going to need a reachabilityFence in many (almost all?) methods of
classes with finalizers.

Andrew.
_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Andrew Haley
In reply to this post by Andrew Haley
I'm sorry, that reply was unnecessarily harsh.  However, I do
this that such speculation is dangerous.

Andrew.
_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Vitaly Davidovich
In reply to this post by Andrew Haley

We did discuss it, and on that thread an Oracle engineer said there's no plan to do that.  In addition, there's a JBS entry to optimize empty method calls by removing prologue and epilogue, with reachabilityFence as the motivator for that.  Correctness does come first, but these are orthogonal/parallel concerns: someone has to update JDK code with this API and someone else has to lower the call in the optimizer.  I don't think this call can/should be liberally sprinkled in performance sensitive code if it actually emits a call, this shouldn't happen from day 1.

sent from my phone

On Dec 8, 2015 4:50 AM, "Andrew Haley" <[hidden email]> wrote:
On 07/12/15 15:20, Vitaly Davidovich wrote:
> If reachabilityFence use is going to proliferate, especially in perf
> sensitive places, Hotspot will need to make this method simply a
> liveness marker and not emit a call like the current
> prototype/version is doing.

Perhaps, but we've already discussed that, and it's not so hard.
Surely correctness comes first, then we optimize.  I expect we're
going to need a reachabilityFence in many (almost all?) methods of
classes with finalizers.

Andrew.

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Vitaly Davidovich
In reply to this post by Andrew Haley

sent from my phone
On Dec 8, 2015 4:58 AM, "Andrew Haley" <[hidden email]> wrote:
>
> On 07/12/15 19:17, Vitaly Davidovich wrote:
> >>
> >> Correct me if I'm wrong, but basically what you are saying is that hotspot
> >> does not perform advanced enough analysis to see that 'this' is actually
> >> unnecessary after the call to unsafe.
> >
> >
> > In which case? In DBB, `this` is still necessary after the Unsafe call
> > since it needs to update position.  In theory, it could move position
> > update prior to calling Bits.copyToArray() but it would have to prove that
> > it's safe to do so.
>
> That's not at all hard for HotSpot to do.
>
> > Part of that safety check would require it to reason about memory
> > effects, side effects, and failure cases (i.e. position update isn't
> > reached if there's an exception,
>
> What exception?  HotSpot knows everything about Bits.copyToArray()
> because it is inlined.  It knows about all the arguments and
> eliminates bounds checks.
>
> > it would need to preserve that effect, also taking into account that
> > it may not be able to speculatively write to memory and then undo
> > those effects in case of exception).  So the answer is probably
> > "yes" to your question above, in a theoretical sense only.
>
> Not at all.
>
> > I think the reasoning here is similar to the above.
> > ByteBuffer.get() is invoked on a DBB instance, compiler sees a call
> > to Unsafe.getXXX at some point.  The "natural" lifetime of the
> > instance survives until the instance method returns.
>
> No, this is completely wrong.
>
> > So if compiler now wanted to shorten this by observing that `this`
> > is unreachable sooner than method exit, it has to prove a bunch of
> > things like I mentioned above.
>
> Now inline everything.
>
> Please stop sepeculating about what an optimizer can and can't do.
> Please instead concerntrate on the semantics of the language and think
> about correctness.
>
> Andrew.
> _______________________________________________
> Concurrency-interest mailing list
> [hidden email]
> http://cs.oswego.edu/mailman/listinfo/concurrency-interest


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Vitaly Davidovich
In reply to this post by Andrew Haley

sent from my phone
On Dec 8, 2015 4:58 AM, "Andrew Haley" <[hidden email]> wrote:
>
> On 07/12/15 19:17, Vitaly Davidovich wrote:
> >>
> >> Correct me if I'm wrong, but basically what you are saying is that hotspot
> >> does not perform advanced enough analysis to see that 'this' is actually
> >> unnecessary after the call to unsafe.
> >
> >
> > In which case? In DBB, `this` is still necessary after the Unsafe call
> > since it needs to update position.  In theory, it could move position
> > update prior to calling Bits.copyToArray() but it would have to prove that
> > it's safe to do so.
>
> That's not at all hard for HotSpot to do.
>
> > Part of that safety check would require it to reason about memory
> > effects, side effects, and failure cases (i.e. position update isn't
> > reached if there's an exception,
>
> What exception?  HotSpot knows everything about Bits.copyToArray()
> because it is inlined.  It knows about all the arguments and
> eliminates bounds checks.

This was a general statement about optimization.  By exception, I really mean control flow.

>
> > it would need to preserve that effect, also taking into account that
> > it may not be able to speculatively write to memory and then undo
> > those effects in case of exception).  So the answer is probably
> > "yes" to your question above, in a theoretical sense only.
>
> Not at all.

Anything more substantive here?

>
> > I think the reasoning here is similar to the above.
> > ByteBuffer.get() is invoked on a DBB instance, compiler sees a call
> > to Unsafe.getXXX at some point.  The "natural" lifetime of the
> > instance survives until the instance method returns.
>
> No, this is completely wrong.

Same here

>
> > So if compiler now wanted to shorten this by observing that `this`
> > is unreachable sooner than method exit, it has to prove a bunch of
> > things like I mentioned above.
>
> Now inline everything.
>
> Please stop sepeculating about what an optimizer can and can't do.
> Please instead concerntrate on the semantics of the language and think
> about correctness.

It's speculation based on observing a real compiler and looking at its code generation in various scenarios.  The bottom line is that it plays conservative a lot of the time, and that is the sentiment I've picked up from various conversations on the compiler list over the years.  Yes, it's speculation and a Sufficiently Smart Compiler can do things differently.  But keep in mind if hotspot started scheduling operations more aggressively around Unsafe usage a lot of code would break.

>
> Andrew.
> _______________________________________________
> Concurrency-interest mailing list
> [hidden email]
> http://cs.oswego.edu/mailman/listinfo/concurrency-interest


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Paul Sandoz
In reply to this post by Vitaly Davidovich

On 8 Dec 2015, at 12:56, Vitaly Davidovich <[hidden email]> wrote:

We did discuss it, and on that thread an Oracle engineer said there's no plan to do that. 


That was me :-) My response was a little more nuanced:


AFAIK there are no plans to do such expansion.

(I may have been misunderstanding what you meant by “expansion”.)

First i wanna get it into JDK 9 as currently optimized (via no inlining), and then iterate e.g. tackling https://bugs.openjdk.java.net/browse/JDK-8130398 if that optimization route is appropriate, and updating various areas in the JDK to use it (some sooner that others perhaps, depending on performance requirements). 

Paul.


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest

signature.asc (858 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Vitaly Davidovich
That was me :-) My response was a little more nuanced:

Ok Paul, I tried to keep this anonymous :)

   http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2015-November/020006.html
AFAIK there are no plans to do such expansion.
(I may have been misunderstanding what you meant by “expansion”.)

Ok.

First i wanna get it into JDK 9 as currently optimized (via no inlining), and then iterate e.g. tackling https://bugs.openjdk.java.net/browse/JDK-8130398 if that optimization route is appropriate, and updating various areas in the JDK to use it (some sooner that others perhaps, depending on performance requirements).

That sounds good.  Removing prologue/epilogue for empty methods sounds like a good idea generally, but at the risk of repeating myself, ideally there's no call at all.  Andrew thinks it shouldn't be difficult, so I'd volunteer him for it. :)



On Tue, Dec 8, 2015 at 8:40 AM, Paul Sandoz <[hidden email]> wrote:

On 8 Dec 2015, at 12:56, Vitaly Davidovich <[hidden email]> wrote:

We did discuss it, and on that thread an Oracle engineer said there's no plan to do that. 


That was me :-) My response was a little more nuanced:


AFAIK there are no plans to do such expansion.

(I may have been misunderstanding what you meant by “expansion”.)

First i wanna get it into JDK 9 as currently optimized (via no inlining), and then iterate e.g. tackling https://bugs.openjdk.java.net/browse/JDK-8130398 if that optimization route is appropriate, and updating various areas in the JDK to use it (some sooner that others perhaps, depending on performance requirements). 

Paul.


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest



_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Andrew Haley
In reply to this post by Vitaly Davidovich
On 12/08/2015 12:07 PM, Vitaly Davidovich wrote:
>>> I think the reasoning here is similar to the above.
>>> ByteBuffer.get() is invoked on a DBB instance, compiler sees a call
>>> to Unsafe.getXXX at some point.  The "natural" lifetime of the
>>> instance survives until the instance method returns.
>>
>> No, this is completely wrong.

> Same here

The lifetime, natural or otherwise, of an instance does not survive
until an instance method returns because, a lot of the time, that
instance method is inlined.

> It's speculation based on observing a real compiler and looking at
> its code generation in various scenarios.  The bottom line is that
> it plays conservative a lot of the time, and that is the sentiment
> I've picked up from various conversations on the compiler list over
> the years.  Yes, it's speculation and a Sufficiently Smart Compiler
> can do things differently.

It's not just HotSpot, though: some VMs are even more aggressive, and
we have seen finalizers executed even before constructors have
completed.  And that is allowed by the specification.

> But keep in mind if hotspot started scheduling operations more
> aggressively around Unsafe usage a lot of code would break

It's already broken.

Andrew.
_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|

Re: DirectByteBuffers and reachabilityFence

Vitaly Davidovich
The lifetime, natural or otherwise, of an instance does not survive
until an instance method returns because, a lot of the time, that
instance method is inlined.

You're talking about optimization here (inlining); by "natural" I meant the naive/no optimization case (e.g. interpreter, debugger attached w/breakpoint in method, etc).

It's not just HotSpot, though: some VMs are even more aggressive, and
 
Which java VMs are these? Just curious.
 
we have seen finalizers executed even before constructors have
completed.  And that is allowed by the specification.

Ok, but that's beside the point, really.  Surely if compiler can optimize and arrange for liveness to allow for it, then it's a good thing it does that.  My point isn't that this cannot happen due to spec, but rather that in places like DBB where `this` is used after the Unsafe call the  compiler has to schedule things differently in order to reduce lifetime.  And my point is that compilers generally tend to be cautious in doing things that may break code.  This is the practical aspect we were referring to - it's actual humans writing these optimizations, and they're sensitive to breaking code, particularly in java.  Theoretically, yes, anything is possible.

It's already broken.

Sure.  Now try to submit a patch to Hotspot that will break this case, even if allowed by spec, and see how far you get :). 
 

On Tue, Dec 8, 2015 at 9:33 AM, Andrew Haley <[hidden email]> wrote:
On 12/08/2015 12:07 PM, Vitaly Davidovich wrote:
>>> I think the reasoning here is similar to the above.
>>> ByteBuffer.get() is invoked on a DBB instance, compiler sees a call
>>> to Unsafe.getXXX at some point.  The "natural" lifetime of the
>>> instance survives until the instance method returns.
>>
>> No, this is completely wrong.

> Same here

The lifetime, natural or otherwise, of an instance does not survive
until an instance method returns because, a lot of the time, that
instance method is inlined.

> It's speculation based on observing a real compiler and looking at
> its code generation in various scenarios.  The bottom line is that
> it plays conservative a lot of the time, and that is the sentiment
> I've picked up from various conversations on the compiler list over
> the years.  Yes, it's speculation and a Sufficiently Smart Compiler
> can do things differently.

It's not just HotSpot, though: some VMs are even more aggressive, and
we have seen finalizers executed even before constructors have
completed.  And that is allowed by the specification.

> But keep in mind if hotspot started scheduling operations more
> aggressively around Unsafe usage a lot of code would break

It's already broken.

Andrew.


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
1234 ... 8