JDK-8152271 : MemberNameTable doesn't purge stale entries
  • Type: Bug
  • Component: hotspot
  • Sub-Component: jvmti
  • Affected Version: 7,8,9
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: x86_64
  • Submitted: 2016-03-15
  • Updated: 2017-04-07
  • Resolved: 2016-06-15
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 9
9 b126Fixed
Related Reports
Cloners :  
Relates :  
Relates :  
Description
FULL PRODUCT VERSION :


A DESCRIPTION OF THE PROBLEM :
Calling findSpecial is leaking memory.

THE PROBLEM WAS REPRODUCIBLE WITH -Xint FLAG: Did not try

THE PROBLEM WAS REPRODUCIBLE WITH -server FLAG: Yes

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
import java.lang.invoke.*;

public class Leak {
  public void callMe() {
  }
  
  public static void main(String[] args) throws Throwable {
    Leak leak = new Leak();
    
    while(true) {
      MethodHandles.Lookup lookup = MethodHandles.lookup();
      MethodType mt = MethodType.fromMethodDescriptorString("()V", Leak.class.getClassLoader());
      // findSpecial leaks some native mem
      MethodHandle mh = lookup.findSpecial(Leak.class, "callMe", mt, Leak.class);
      mh.invokeExact(leak);
    }
  }
}

EXPECTED VERSUS ACTUAL BEHAVIOR :
Expected: No increase in memory
Result: Increase in memory
REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
import java.lang.invoke.*;

public class Leak {
  public void callMe() {
  }
  
  public static void main(String[] args) throws Throwable {
    Leak leak = new Leak();
    
    while(true) {
      MethodHandles.Lookup lookup = MethodHandles.lookup();
      MethodType mt = MethodType.fromMethodDescriptorString("()V", Leak.class.getClassLoader());
      // findSpecial leaks some native mem
      MethodHandle mh = lookup.findSpecial(Leak.class, "callMe", mt, Leak.class);
      mh.invokeExact(leak);
    }
  }
}
---------- END SOURCE ----------


Comments
This change is being re-applied to jdk9 in JDK-8162795, with a better solution for jdk10 planned in JDK-8174749.
21-02-2017

The backout bug in jdk9 was: https://bugs.openjdk.java.net/browse/JDK-8161445
17-02-2017

On 6/17/16 7:08 AM, Vladimir Ivanov wrote: > I've checked this in but something in your mail made me worry. > > If I intern a MemberName, in this table, it's not immutable, is it? So > some code creates membername1, some other code gets membername1 and then > the second bit of code modified membername1, which could mess up the > first one? > > Or is the place where I've done the interning safe because the > MemberName is not changed at that point? Good point, Coleen. I don't see any scenario where it can happen now, but I agree that current implementation is fragile. It works now, because (1) MN changes are limited (MNs are effectively immutable once they are published); and (2) cloning doesn't intern instances. So, while freshly allocated instance is adjusted it isn't interned (since it is guarded by the original one). I see only 1 problematic method, but it's not used: jdk/src/java.base/share/classes/java/lang/invoke/MemberName.java: public MemberName getDefinition() { if (!isResolved()) throw new IllegalStateException("must be resolved: "+this); if (isType()) return this; MemberName res = this.clone(); res.clazz = null; res.type = null; res.name = null; res.resolution = res; res.expandFromVM(); assert(res.getName().equals(this.getName())); return res; } Actually, MN expansion (filling fields in resolved MN from vmtarget) makes interning harder, but I don't see it used anywhere. I vaguely remember it is needed for bootstrapping purposes, but I can't find any usages now. Best regards, Vladimir Ivanov
05-08-2016

This fix caused this regression in performance. It doesn't look like searching lists is helpful if the JDK code is going to clone MemberName and mutate them. Also, this fix assumes that MemberName are immutable but that's not really a safe assumption long term.
28-07-2016

Thanks Eric for running the tests.
09-06-2016

Sure I will run that over the weekend.
03-06-2016

[~shade] [~ecaspole] Do you have performance tests that I can run on my changes to the member name table? thanks.
03-06-2016

With a little bit of extra code, I implemented interning MemberNames in the jvm. This works perfectly for this case but this case is not representative. Running just the jmod builds in the build, there are much fewer reuses of MemberName. Also, MemberName contains flags that represent CallInfo, so just matching the Method* isn't sufficient. Do you have a good test case or code to run performance testing on?
01-06-2016

[~sspitsyn], [~coleenp], may I ask you to look into the issue? It's a native memory leak in InstanceKlass::_member_names. The purging happens only when InstanceKlass is deallocated. But if a user does excessive lookups, the table is overflown with numerous stale entries. As an interim fix, stale slot reuse logic should help alleviate a problem: diff --git a/src/share/vm/prims/methodHandles.cpp b/src/share/vm/prims/methodHandles.cpp --- a/src/share/vm/prims/methodHandles.cpp +++ b/src/share/vm/prims/methodHandles.cpp @@ -1023,7 +1023,16 @@ void MemberNameTable::add_member_name(jweak mem_name_wref) { assert_locked_or_safepoint(MemberNameTable_lock); - this->push(mem_name_wref); + int idx = 0; + for (; idx < length(); idx++) { + jweak ref = this->at(idx); + oop mem_name = JNIHandles::resolve(ref); + if (mem_name == NULL) { + JNIHandles::destroy_weak_global(ref); + break; // reuse the emtpy slot + } + } + this->at_put_grow(idx, mem_name_wref); } #if INCLUDE_JVMTI But it doesn't completely solve the problem, since the array never shrinks and its actual size depends on the frequency of the GCs (how frequently the clears slots). As a proper fix, MemberName interning in _member_names (proposed in JDK-8013267) should completely solve the problem.
21-03-2016

Yes, it is the problem: (lldb) p *_member_names (MemberNameTable) $5 = { GrowableArray<_jobject *> = { ... _len = 108546 _max = 196608 ... } } }
21-03-2016

The most probable culprit is InstanceKlass::add_member_name() which unconditionally registers MemberNames in MemberNameTable, but never compacts the backing array.
21-03-2016

Mac OS X analyses (profiling results) for the original Leak and an alternative Leak-with-findVirtual attached. Leak-with-findVirtual also shows increasing memory usage; it does not seem to be as steep as in the findSpecial case.
21-03-2016

The j.l.invoke folk may have a better idea where to start with this - moved.
21-03-2016

below are the results 7u55 - Pass 7u60 - Fail 7u80 - Fail 7u101 - Fail 8u0 - Fail 8u74 - Fail 9 ea b 110 - Fail
21-03-2016

Memory leak introduced in 7u60 build, it exists in all the latest builds. (7u80, 8u74 and 9 ea b-110) Below are the results 1. Executing below commands in one terminal -sh-4.1$ /opt/java/jdk1.8.0_74/bin/javac Leak.java -sh-4.1$ /opt/java/jdk1.8.0_74/bin/java Leak 2. Executing date followed by free command in aother terminal -sh-4.1$ date Mon Mar 21 06:39:51 GMT 2016 -sh-4.1$ free -m total used free shared buffers cached Mem: 15211 7143 8067 0 944 4408 -/+ buffers/cache: 1791 13420 Swap: 15575 18 15557 -sh-4.1$ -sh-4.1$ date Mon Mar 21 06:40:06 GMT 2016 -sh-4.1$ free -m total used free shared buffers cached Mem: 15211 7150 8061 0 944 4408 -/+ buffers/cache: 1797 13413 Swap: 15575 18 15557 -sh-4.1$ date Mon Mar 21 06:40:14 GMT 2016 -sh-4.1$ free -m total used free shared buffers cached Mem: 15211 7720 7490 0 944 4408 -/+ buffers/cache: 2368 12843 Swap: 15575 18 15557 -sh-4.1$ date Mon Mar 21 06:40:19 GMT 2016 -sh-4.1$ free -m total used free shared buffers cached Mem: 15211 8095 7116 0 944 4408 -/+ buffers/cache: 2743 12468 Swap: 15575 18 15557 -sh-4.1$ date Mon Mar 21 06:40:22 GMT 2016 -sh-4.1$ free -m total used free shared buffers cached Mem: 15211 8587 6624 0 944 4408 -/+ buffers/cache: 3234 11977 Swap: 15575 18 15557 -sh-4.1$ date Mon Mar 21 06:40:27 GMT 2016 -sh-4.1$ free -m total used free shared buffers cached Mem: 15211 8711 6500 0 944 4408 -/+ buffers/cache: 3358 11853 Swap: 15575 18 15557 -sh-4.1$ date Mon Mar 21 06:40:31 GMT 2016 -sh-4.1$ free -m total used free shared buffers cached Mem: 15211 8784 6427 0 944 4408 -/+ buffers/cache: 3431 11780 Swap: 15575 18 15557 -sh-4.1$ free -m total used free shared buffers cached Mem: 15211 9146 6065 0 944 4408 -/+ buffers/cache: 3793 11418 Swap: 15575 18 15557 -sh-4.1$ date Mon Mar 21 06:40:59 GMT 2016 -sh-4.1$ free -m total used free shared buffers cached Mem: 15211 9193 6018 0 944 4408 -/+ buffers/cache: 3840 11370 Swap: 15575 18 15557 -sh-4.1$ date Mon Mar 21 06:43:25 GMT 2016 -sh-4.1$ free -m total used free shared buffers cached Mem: 15211 11451 3760 0 944 4408 -/+ buffers/cache: 6098 9113 Swap: 15575 18 15557
21-03-2016