[concurrency-interest] DirectByteBuffers and reachabilityFence

Vitaly Davidovich vitalyd at gmail.com
Mon Dec 7 14:17:26 EST 2015


>
> 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 <adc at quartetfs.com>
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 <vitalyd at gmail.com>
> 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 <
>> adc at quartetfs.com> 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 <vitalyd at gmail.com>
>>> 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 <
>>>> adc at quartetfs.com> 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:
>>>>> #   http://bugreport.java.com/bugreport/crash.jsp
>>>>> #
>>>>>  * </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 <vitalyd at gmail.com>
>>>>> 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" <adc at quartetfs.com>
>>>>>> 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
>>>>>>> Concurrency-interest at cs.oswego.edu
>>>>>>> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Alexandre
>>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> Alexandre
>>>
>>
>>
>
>
> --
> Alexandre
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://cs.oswego.edu/pipermail/concurrency-interest/attachments/20151207/92a02c03/attachment-0001.html>


More information about the Concurrency-interest mailing list