On Mon, 2 Feb 2026 15:44:21 GMT, Tobias Hartmann <[email protected]> wrote:

> When detecting a mismatch in the calling convention, we call 
> `super_method->set_mismatch()` out of the scope of the `Compile_lock`. When 
> class loading/linking happens in parallel, other dependencies might be 
> invalidated and nmethods will be marked for deoptimization. Verification code 
> will then check that all nmethods that have violated dependencies have been 
> marked. The verification will fail once it finds a nmethod that now has a 
> violated dependency not due to the class loading but due to the mismatch that 
> we just set.
> 
> Similar to how the [runtime handles similar 
> situations](https://github.com/openjdk/valhalla/blob/2ad3ce1646bb5c717239f683686dd1034a9340c1/src/hotspot/share/oops/instanceKlass.cpp#L1230-L1235),
>  `set_mismatch` needs to be performed while holding the lock. We then keep 
> the lock until we marked all methods.
> 
> When looking at the code, I noticed that more instructions can be moved out 
> of the scope of the lock, so I did that as well.
> 
> I quickly looked into creating a regression test for this but it's 
> non-trivial. Also, our internal testing triggers this reliably, so I decided 
> to go without a regression test for now.
> 
> Thanks,
> Tobias

Seems reasonable. As far as I can tell, I think what is not in the lock scope 
is fine. And I've checked it's the only place we use `Method::set_mismatch()`.

src/hotspot/share/runtime/sharedRuntime.cpp line 2943:

> 2941:                   methodHandle mh(thread, super_method);
> 2942:                   DeoptimizationScope deopt_scope;
> 2943:                   {

Maybe one could put a comment on the block start, stating that's it's to limit 
the scope of the MutexLocker. I've seen such pattern, so it's possible that I'd 
guess it out of context, but uncertain. I've seen some blocks whose purpose 
were sometimes unclear (either to allow to shadow variables, or maybe a 
leftover of some control structure).

But maybe I'm being over cautious, and documenting a common pattern is not 
worth the noise. I'm not convinced myself. Feel free to ignore.

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

Marked as reviewed by mchevalier (Committer).

PR Review: 
https://git.openjdk.org/valhalla/pull/2014#pullrequestreview-3744995441
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2014#discussion_r2758843106

Reply via email to