Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JVM crash: org.lwjgl.system.MemoryUtil reaches after the end of the object #491

Closed
shipilev opened this issue Aug 8, 2019 · 6 comments
Closed

Comments

@shipilev
Copy link

shipilev commented Aug 8, 2019

Happens in current Git tip with fastdebug OpenJDK binaries. I use my own builds below, but this can be reproduced with self-built OpenJDK binaries too.

$ curl https://builds.shipilev.net/openjdk-jdk/openjdk-jdk-latest-linux-x86_64-fastdebug.tar.xz | tar xJf -
$ export JAVA_HOME=`pwd`/jdk
$ export PATH=$JAVA_HOME/bin:$PATH
$ java -version
openjdk version "14-testing" 2020-03-17
OpenJDK Runtime Environment (fastdebug build 14-testing+0-builds.shipilev.net-openjdk-jdk-b959-20190807-jdk-1332)
OpenJDK 64-Bit Server VM (fastdebug build 14-testing+0-builds.shipilev.net-openjdk-jdk-b959-20190807-jdk-1332, mixed mode, sharing)
$ ant

...
tests:
    [Tests] [LWJGL] Version: 3.2.3 SNAPSHOT
    [Tests] [LWJGL] 	 OS: Linux v4.15.0-52-generic
    [Tests] [LWJGL] 	JRE: 14-testing amd64
    [Tests] [LWJGL] 	JVM: OpenJDK 64-Bit Server VM v14-testing+0-builds.shipilev.net-openjdk-jdk-b959-20190807-jdk-1332 by Aleksey Shipilev
    [Tests] # To suppress the following error report, specify this argument
    [Tests] # after -XX: or in .hotspotrc:  SuppressErrorAt=/unsafe.cpp:119
    [Tests] #
    [Tests] # A fatal error has been detected by the Java Runtime Environment:
    [Tests] #
    [Tests] #  Internal Error (/home/buildbot/worker/jdkX-linux/build/src/hotspot/share/prims/unsafe.cpp:119), pid=9818, tid=9821
    [Tests] #  assert(byte_offset < p_size) failed: Unsafe access: offset 56 > object's size 56
    [Tests] #
    [Tests] # JRE version: OpenJDK Runtime Environment (14.0) (fastdebug build 14-testing+0-builds.shipilev.net-openjdk-jdk-b959-20190807-jdk-1332)
    [Tests] # Java VM: OpenJDK 64-Bit Server VM (fastdebug 14-testing+0-builds.shipilev.net-openjdk-jdk-b959-20190807-jdk-1332, mixed mode, sharing, tiered, compressed oops, g1 gc, linux-amd64)
    [Tests] # Problematic frame:
    [Tests] # V  [libjvm.so+0x18482d4]  assert_field_offset_sane(oop, long)+0xe4

The top of the stack is:

Stack: [0x00007fa8c3a22000,0x00007fa8c3a63000],  sp=0x00007fa8c3a5ed60,  free space=243k
Native frames: (J=compiled Java code, A=aot compiled Java code, j=interpreted, Vv=VM code, C=native code)
V  [libjvm.so+0x18482d4]  assert_field_offset_sane(oop, long)+0xe4
V  [libjvm.so+0x1852483]  Unsafe_GetInt+0xf3
J 552  jdk.internal.misc.Unsafe.getInt(Ljava/lang/Object;J)I java.base@14-testing (0 bytes) @ 0x00007fa8ab700b18 [0x00007fa8ab7009c0+0x0000000000000158]
j  sun.misc.Unsafe.getInt(Ljava/lang/Object;J)I+5 jdk.unsupported@14-testing
j  org.lwjgl.system.MemoryUtil.getParentOffset(ILjava/nio/Buffer;Ljava/util/function/Function;)J+50
j  org.lwjgl.system.MemoryUtil.<clinit>()V+220
v  ~StubRoutines::call_stub

So I think org.lwjgl.system.MemoryUtil.getParentOffset reaches to something after the end of the object. This is incorrect and caught by JVM assert. It could have crashed the JVM without the assert, especially in release builds.

@shipilev
Copy link
Author

shipilev commented Aug 8, 2019

Originally found by running Minecraft with fastdebug binaries:
https://mail.openjdk.java.net/pipermail/shenandoah-dev/2019-August/010333.html

@Spasi Spasi added the Type: Bug label Aug 8, 2019
@Spasi
Copy link
Member

Spasi commented Aug 8, 2019

Thank you @shipilev,

Tracked it down to JDK-8208362. I was actually aware of the old behavior and found it unreasonable, that's why the custom slice/duplicate implementations in LWJGL do the same thing as JDK 12, i.e. attach the source buffer's attachment. But I never made the connection that it's an implementation detail that, if changed, would bite us in the offset detection code.

I've implemented getParentOffset differently now and the problem will be fixed in the next 3.2.3 snapshot. All tests pass on JDK 12-14 fastdebug builds.

Btw, field offsets are detected using Unsafe so that users can drop LWJGL in modular applications without having to mess with JVM arguments to allow reflective access to java.nio internals.

@Spasi Spasi closed this as completed in 0e13a49 Aug 8, 2019
@shipilev
Copy link
Author

shipilev commented Aug 8, 2019

Interesting. Does this mean JDK-8208362 should be backported to 11u and 8u to make LWJGL use it there as well?

@Spasi
Copy link
Member

Spasi commented Aug 8, 2019

LWJGL does not require JDK-8208362, it works fine on either 8-11 or 12+ after the above fix.

If backported, LWJGL users migrating to the newer JDK would also need to update to latest LWJGL. The bug exists since 3.2.1, which is the version Minecraft uses afaik.

@shipilev
Copy link
Author

shipilev commented Aug 8, 2019

Pity. So if JDK-8208362 enters newer 11u or 8u, then current Minecraft running with those new 11u or 8u would crash until it is upgraded to LWJGL 3.2.3+. That means the less risky way would be waiting for LWJGL-dependent projects to make the update, and then consider backporting the JDK change that resolves the memory leak (for everyone, not only LWJGL users).

@Spasi
Copy link
Member

Spasi commented Aug 8, 2019

I would generally be in favor of going ahead with the backport, the JDK has top priority and LWJGL can handle the support burden.

But in this case I would also argue that the JDK-8208362 fix is nowhere near critical. The leak can be demonstrated trivially, but real-life nio code doesn't create such long attachment chains.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants