RFR(S): 8028107: Kitchensink crashed with EAV

[Christian Thalinger]

Do I understand the change in compiledIC.cpp right that this makes sure we fall-back to the interpreter when the patching in SharedRuntime::resolve_sub_helper didn’t happen?

On Dec 3, 2013, at 6:25 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

> http://cr.openjdk.java.net/~kvn/8028107/webrev/
> 
> https://bugs.openjdk.java.net/browse/JDK-8028107
> 
> As I explained in previous mail SharedRuntime::resolve_sub_helper() has Lock between saving nmethod pointer in local CompiledICInfo variable and when it is used to create new compiledIC entry.
> Lock may block for safepoint during which nmethod can change state: it can be deoptimized or unloaded.
> I looked in other places and did not find anything suspicious. For example, in handle_ic_miss_helper() compiled entry computation and compiledIC setting are done under the same lock.
> 
> The fix is to check the state of caller and callee nmethods and skip call site patching if any of them is not alive.
> 
> Note, we have strange/swapped names for functions which check nmethod state:
> 
>  bool  is_in_use() const { return _state == alive; }
>  bool  is_alive() const  { return _state == alive || _state == not_entrant; }
> 
> The dest_entry_point assert in resolve_sub_helper() is mostly useless now but I kept it slightly modified.
> 
> I was not able to reproduce the problem running kitchensink for 5 days. So I can't verify this fix. As I pointed in the bug report this failure happens about once per year.
> 
> I also fixed indention in nmethod.cpp but webrev does not handle such changes. Here they are:
> 
> src/share/vm/code/nmethod.cpp
> @@ -1660,8 +1660,8 @@
>           CompiledICHolder* cichk_oop = ic->cached_icholder();
>           if (cichk_oop->holder_method()->method_holder()->is_loader_alive(is_alive) &&
>               cichk_oop->holder_klass()->is_loader_alive(is_alive)) {
> -              continue;
> -            }
> +            continue;
> +          }
>         } else {
>           Metadata* ic_oop = ic->cached_metadata();
>           if (ic_oop != NULL) {
> @@ -1677,8 +1677,8 @@
>               ShouldNotReachHere();
>             }
>           }
> -          }
> -          ic->set_to_clean();
> +        }
> +        ic->set_to_clean();
>       }
>     }
>   }
> 
> Thanks,
> Vladimir