RFR: 8322743: assert(held_monitor_count() == jni_monitor_count()) failed [v2]

[Vladimir Kozlov]

On Fri, 19 Jan 2024 14:42:22 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Vladimir Kozlov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Address review comments
>
> Ok, so I did some rudimentary study of EA. And now this PR makes much more sense ;)
> 
> Let me summarize my understanding of the issue:
> An object gets allocated in interpreter, and we lock on it in the interpreter.
> OSR is triggered, the object is passed in as OSR parameter, we hold the lock.
> The OSR control flow now looks like this:
> 
> 
> StartOSR:
>   LoadP -> load the object created in interpreter
>   we have not_global_escape(LoadP) == false
>   so this is correctly marked as escaping
>   now the osr path injects into the middle of the loop
> 
> Loop:
>   Phi -> merge interpreter obj and that from this compiled code
>   we have not_global_escape(Phi) == false
>   ...
>   Unlock(Phi)
>   ...
>   check some condition, maybe return
>   ...
>   obj = CheckCastPP( Allocate(i.e. new Object()) )
>   we have not_global_escape(obj) == true
>   this is correct, the object will never escape
>   Lock(obj)
>   ...
>   goto Loop
> 
> 
> So if I understand this correctly, the marking in/with the ConnectionGrap is correct:
> - The object passed in through OSR is marked as escaping.
> - The object created locally is marked as non-escaping.
> - The loop-phi that merges the two must therefore also be possibly escaping.
> 
> The question is then with the condition of Lock removal:
> Can we remove the lock, just because its object is marked as non-escaping?
> At first glance: obviously, because nobody else could ever have the object, and so nobody can ever lock/unlock it.
> 
> In the example, if we look at the Unlock node, we cannot remove it (at least at first):
> its object is possibly escaping, because the Phi is not marked non-escaping.
> But we can remove the Lock, since its object is non-escaping.
> This is where the trouble starts.
> 
> I think it is exactly for this reason, that @vnkozlov thinks one cannot just look at the object of the individual Lock/Unlock node, but one has to look at all Lock/Unlock nodes of a BoxLock, and see if all objects are non-escaping.
> 
> @vnkozlov please correct me if I got something wrong ;)
> 
> I was trying to see what the meaning of the BoxLockNode is, but I did not find any useful documentation. Can you help me out here? Your patch assumes that all "relevant" Lock/Unlock nodes share the same BoxLockNode. Why is that the case?

@eme64, @iwanowww and @dean-long please look.

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

PR Comment: https://git.openjdk.org/jdk/pull/17331#issuecomment-1969983751