JDK-8042235 : redefining method used by multiple MethodHandles crashes VM
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 7u45,8,9
  • Priority: P1
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2014-04-30
  • Updated: 2019-09-13
  • Resolved: 2014-11-19
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 7 JDK 8 JDK 9
7u80Fixed 8u40Fixed 9 b42Fixed
Related Reports
Duplicate :  
JDK-8013267 - move MemberNameTable from native code to Java heap, use to intern MemberNames
Relates :  
JDK-8133612 - new clone logic added in 8042235 is missing from compiler intrinsics
Relates :  
JDK-8027162 - JSR292: implement MemberName interning
Relates :  
JDK-8008511 - JSR 292: MemberName vmtarget refs to methods must be updated at class redefinition
Relates :  
JDK-8013267 - move MemberNameTable from native code to Java heap, use to intern MemberNames
Description
This is a bug found by [~dnsimon].  The attached test case creates two MethodHandles for the same method, redefines the method and then calls the first created MethodHandle:

        // fooMH2 displaces fooMH1 from the MemberNamesTable
        MethodHandle fooMH1 = lookup.unreflect(fooMethod);
        MethodHandle fooMH2 = lookup.unreflect(fooMethod);

        // Redefining Foo.getName() causes vmtarget to be updated
        // in fooMH2 but not fooMH1
        redefineFoo();

        // Full GC causes fooMH1.vmtarget to be deallocated
        System.gc();

        // Calling fooMH1.vmtarget crashes the VM
        System.out.println("fooMH1.invoke = " + fooMH1.invokeExact());

The problem is that MemberNameTable only stores the last created MemberName for a given method.

$ java RedefineMethodUsedByMultipleMethodHandles
fooMH1.invoke = foo
fooMH2.invoke = foo
objc[11873]: Class JavaLaunchHelper is implemented in both /Users/cthaling/build/jdk9/hs-comp/hotspot/jdk-universal/bin/java and /Users/cthaling/build/jdk9/hs-comp/hotspot/jdk-universal/jre/lib/libinstrument.dylib. One of the two will be used. Which one is undefined.
redefining class RedefineMethodUsedByMultipleMethodHandles$Foo
replacing "foo" with "bar"
#
# A fatal error has been detected by the Java Runtime Environment:
#
#  SIGSEGV (0xb) at pc=0x0000000000000000, pid=11873, tid=6147
#
# JRE version: Java(TM) SE Runtime Environment (9.0-b04) (build 1.9.0-ea-b04)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (25.0-b62-internal-debug mixed mode bsd-amd64 compressed oops)
# Problematic frame:
# 
[error occurred during error reporting (printing problematic frame), id 0xe0000000]

# 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:
# /Users/cthaling/Downloads/hs_err_pid11873.log
#
# If you would like to submit a bug report, please visit:
#   http://bugreport.sun.com/bugreport/crash.jsp
#
Current thread is 6147
Dumping core ...
Abort trap: 6
Comments
Tom, thank you for the comment. Filing a new bug would be very helpful.
12-08-2015
I think there's a lurking problem with this unless it was fixed under in some way I can't see. There's new magic in JVM_Clone for to register clones but clone is intrinsified by C2 and I don't see equivalent logic there or any logic to gate the intrinsic. So once method handle code is warmed up enough to be compiled by C2 I think the original issue could reappear. I don't understand the method handle logic well enough to construct a test case that would trigger the clone in a visible way either. Should I file a new bug?
12-08-2015
Thanks, Christian! I agree, fixing the JDK-8013267 looks reasonable.
08-05-2014
Canonicalizing MemberName in native code will mean MemberNameTable can no longer use Method::method_idnum() as an index since there can be multiple different MemberName instances for the one Method*. One solution would be use a dedicated Java type for boxing the Method* and MemberName points to instances of that type. More detail in a comment on JDK-8013267.
01-05-2014
Oh, [~vlivanov] filed the same but a long time ago as JDK-8027162 and it got closed as a duplicate of JDK-8013267. My preference would be to implement JDK-8013267.
01-05-2014
The following output shows how the test is passed with my TMP hack: MY_VM_TRACE: added elem with index: 1 MY_VM_TRACE: added cache with index: 1 fooMH1.invoke = foo fooMH2.invoke = foo redefining class RedefineMethodUsedByMultipleMethodHandles$Foo replacing "foo" with "bar" MY_VM_TRACE: adjusted elem with index: 1 MY_VM TRACE: adjusted cache with index: 1 fooMH2.invoke = bar fooMH1.invoke = bar My TMP hack is this: diff -r 292091fae50a src/share/vm/prims/methodHandles.cpp --- a/src/share/vm/prims/methodHandles.cpp Mon Apr 28 09:31:25 2014 +0000 +++ b/src/share/vm/prims/methodHandles.cpp Thu May 01 01:07:07 2014 -0700 @@ -946,9 +946,22 @@ MemberNameTable::~MemberNameTable() { } } +static jweak tmp_cache[100] = { NULL }; + void MemberNameTable::add_member_name(int index, jweak mem_name_wref) { assert_locked_or_safepoint(MemberNameTable_lock); - this->at_put_grow(index, mem_name_wref); + oop mname0 = (index < _len) ? this->get_member_name(index) : NULL; + if (mname0 == NULL) { + this->at_put_grow(index, mem_name_wref); + if (index == 1) { + printf("MY_VM_TRACE: added elem with index: %d\n", index); fflush(0); + } + } else { + tmp_cache[index] = mem_name_wref; + if (index == 1) { + printf("MY_VM_TRACE: added cache with index: %d\n", index); fflush(0); + } + } } // Return a member name oop or NULL. @@ -991,6 +1004,18 @@ void MemberNameTable::adjust_method_entr oop mem_name = find_member_name_by_method(old_method); if (mem_name != NULL) { java_lang_invoke_MemberName::adjust_vmtarget(mem_name, new_method); + if (j == 1) { + printf("MY_VM_TRACE: adjusted elem with index: %d\n", j); fflush(0); + + if (tmp_cache[j] != NULL) { + oop mem_name = JNIHandles::resolve(tmp_cache[j]); + Method* method = (Method*)java_lang_invoke_MemberName::vmtarget(mem_name); + if (method == old_method) { + java_lang_invoke_MemberName::adjust_vmtarget(mem_name, new_method); + printf("MY_VM_TRACE: adjusted cache with index: %d\n", j); fflush(0); + } + } + } if (RC_TRACE_IN_RANGE(0x00100000, 0x00400000)) { if (!(*trace_name_printed)) {
01-05-2014
Christian, I temporarily re-assigned the bug to you. Please, re-assign it back to me if you prefer to fix the MemberNameTable to store multiple member names for the same function index. Note, this is going to cause some performance degradation.
01-05-2014
I've applied a TMP hacky workaround and proved that this crash happens if more than one member name is associated with the same method. My hack is to cache one more member name for the same index in a static array. The test is passing with this hack. It proves that the test is going to pass if the method's member name is a singleton. One way to fix is be to make the method's member name a singleton which requires to provide this functionality on the Java java.lang.invoke package side. Another way to fix it is to update the MemberName table to store multiple member names for the same method index which is going to be ugly. Christian, how would you prefer to fix it? If the method's member name is supposed to be a singleton then it makes sense to move this bug back to the compiler team. I temporarily moved the bug to the jvmti subcategory.
01-05-2014
ILW=HMH=P1 MethodHandles get used more and more in the core libraries and user code so redefining methods that MethodHandles hold on to is increasingly likely.
30-04-2014