Integrated: 8299074: nmethod marked for deoptimization is not deoptimized

[Tobias Hartmann]

On Mon, 16 Jan 2023 15:51:28 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

> In the failing `UnexpectedDeoptimizationAllTest` there is a race condition between one thread repeatedly calling `WB_DeoptimizeAll()` and the main thread checking nmethod dependencies on class loading and also attempting marking/deoptimization of nmethods due to dependency violations. The problem is that already (concurrently) marked nmethods are not counted and if no new nmethods are marked, we then wrongly assume that there is no need to call `deoptimize_all_marked`:
> https://github.com/openjdk/jdk/blob/08008139cc05a8271e7163eca47d2bc59db2049b/src/hotspot/share/code/codeCache.cpp#L1438-L1440
> 
> Details:
> 
> **Thread 1:** Method `useNativeAccessor(Executable member)` is compiled under the assumption that its argument type `java.lang.reflect.Executable` has only one implementer `java.lang.reflect.Method`. A corresponding dependency is registered in the nmethod and a virtual call to `member.getParameterCount()` is inlined to `java.lang.reflect.Method::getParameterCount()`.
> 
> **Thread 2:** Calls Whitebox API method `WB_DeoptimizeAll` -> `CodeCache::mark_all_nmethods_for_deoptimization()` that marks `useNativeAccessor` for deoptimization.
> 
> **Thread 1:** Triggers class loading of `java.lang.reflect.Constructor` and `CodeCache::flush_dependents_on` -> `CodeCache::mark_for_deoptimization` -> ... -> `DependencyContext::mark_dependent_nmethods` detects that `useNativeAccessor` needs to be deoptimized now that `java.lang.reflect.Executable` has more than one implementer. However, the nmethod is already marked for deoptimization (most nmethods are) and therefore ignored. The marked counter is 0 and therefore `Deoptimization::deoptimize_all_marked()` is not executed either. The thread continues execution and ends up crashing because a `java.lang.reflect.Constructor` object is passed to compiled `useNativeAccessor` which can not handle it (we call the wrong `getParameterCount` on it).
> 
> **Thread 2:** Is still in `WB_DeoptimizeAll`, marking nmethods for deoptimization but didn't get a chance to call `Deoptimization::deoptimize_all_marked()` yet.
> 
> Before [JDK-8221734](https://bugs.openjdk.org/browse/JDK-8221734) in JDK 13, `WB_DeoptimizeAll` acquired the `Compile_lock` but it got [removed](http://hg.openjdk.java.net/jdk/jdk/rev/9b70ebd131b4#l15.7). Instead of re-adding the lock, I modified the code to also count nmethods that were already marked for deoptimization, as suggested by @robehn in the bug comments.
> 
> Thanks,
> Tobias

This pull request has now been integrated.

Changeset: 66f7387b
Author:    Tobias Hartmann <thartmann at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/66f7387b5ffa53861b92b068fb9832fc433d9f79
Stats:     20 lines in 1 file changed: 10 ins; 2 del; 8 mod

8299074: nmethod marked for deoptimization is not deoptimized

Reviewed-by: eosterlund, rehn, kvn

-------------

PR: https://git.openjdk.org/jdk/pull/12012