JDK-8293584 : CodeCache::old_nmethods_do incorrectly filters is_unloading nmethods
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 20
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2022-09-09
  • Updated: 2022-12-20
  • Resolved: 2022-11-21
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 20
20 b25Fixed
Related Reports
Blocks :  
JDK-8295097 - DirectNativeCallWrapper::verify_resolve_call fails with "assert(db != __null && !db->is_adapter_blob()) failed: must use stub!"
Duplicate :  
JDK-8295097 - DirectNativeCallWrapper::verify_resolve_call fails with "assert(db != __null && !db->is_adapter_blob()) failed: must use stub!"
Relates :  
JDK-8295097 - DirectNativeCallWrapper::verify_resolve_call fails with "assert(db != __null && !db->is_adapter_blob()) failed: must use stub!"
Relates :  
JDK-8291830 - jvmti/RedefineClasses/StressRedefine failed: assert(!is_null(v)) failed: narrow klass value can never be zero
Description
Class redefinition needs to scan for all nmethods, including is_unloading ones. If it doesn't, the Method can get nuked and then a concurrent GC will trip on it and crash. The normal iterator used by class redefinition uses the right iterator, but there is an optmized table being used in CodeCache::old_nmethods_do, and it inconsistently does filter is_unloading nmethods, which is a bug.
Comments
Changeset: 08008139 Author: Coleen Phillimore <coleenp@openjdk.org> Date: 2022-11-21 13:46:09 +0000 URL: https://git.openjdk.org/jdk/commit/08008139cc05a8271e7163eca47d2bc59db2049b
21-11-2022
A pull request was submitted for review. URL: https://git.openjdk.org/jdk/pull/11243 Date: 2022-11-18 16:51:45 +0000
18-11-2022
[~eosterlund] I was stuck on writing a test, and forgot about it, but just sent it out for review without one.
18-11-2022
How is it going?
15-11-2022
I don't see any class unloading in JDK-8291830. I'll run something like tier6 on this fix and mark it as noreg-hard.
07-11-2022
I do not have a reproducer. But I tried to reproduce the issue from JDK-8291830 with the "if (!cm->is_unloading())" check removed. I've got two new assert modes that I've not seen before. Please, look at the last two hs_err dumps in my latest comment to this bug. They look interesting. It seems, this test provides a test coverage for this bug: vmTestbase/nsk/jvmti/RedefineClasses/StressRedefine What I see is that the removal of the "if (!cm->is_unloading())" is not enough. (Of course, these two bugs still can be different and independent beasts.) It looks like we sometimes deallocate the same IK more than once. I'm thinking how to construct a trap to understand contexts of the same IK deallocations.
21-10-2022
It's my bug and I believe a one line fix: we should walk all the methods, not just the ones that are not unloading, but if you have a reproducer, that would be great.
20-10-2022
Coleen, I feel that I mistakenly moved this issue to hotspot/runtime. Please, give me more time to look at this issue. It seems to be related to the bug on my plate: JDK-8291830
20-10-2022
The function CodeCache::old_nmethods_do which incorrectly filters is_unloading is: void CodeCache::old_nmethods_do(MetadataClosure* f) { // Walk old method table and mark those on stack. int length = 0; if (old_compiled_method_table != NULL) { length = old_compiled_method_table->length(); for (int i = 0; i < length; i++) { CompiledMethod* cm = old_compiled_method_table->at(i); // Only walk !is_unloading nmethods, the other ones will get removed by the GC. if (!cm->is_unloading()) { <== INCORRECT FILTERING !!! old_compiled_method_table->at(i)->metadata_do(f); } } } . . . } The CodeCache::old_nmethods_do() is only used in the constructor: MetadataOnStackMark::MetadataOnStackMark(bool walk_all_metadata, bool redefinition_walk) { . . . if (walk_all_metadata) { MetadataOnStackClosure md_on_stack; Threads::metadata_do(&md_on_stack); if (redefinition_walk) { // We have to walk the whole code cache during redefinition. CodeCache::metadata_do(&md_on_stack); } else { CodeCache::old_nmethods_do(&md_on_stack); <== THE ONLY CALL TO FUNCTION !!! } . . . } As we can see, it is not a redefinition_walk case. So, I have a doubt this issue is in the JVMTI area. I can remove this check: if (!cm->is_unloading()) { But I have no idea what testing has to be done then. So, I'm moving this issue to hotspot/runtime for initial evaluation. Feel free to move this bug back to JVMTI if necessary.
14-10-2022
Moving from hotspot/runtime -> hotspot/jvmti since this involves JVM/TI RedefineClasses.
09-09-2022