[code-reflection] RFR: Model synchronized statements

[Paul Sandoz]

On Fri, 23 Aug 2024 08:51:56 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Model synchronized statements.
>> 
>> A synchronized op has two bodies one for the expression yielding the monitor and one for the synchronized block that covers the monitor enter and exit.
>> 
>> The lowering of synchronized op produces a model containing monitor enter and exit operations and operations covered by exception regions, ensuring that a monitor exits under exceptional circumstances. 
>> 
>> The interpreter has been updated, but locking is currently on a best effort basis using `ReentrantLock` instances thus we cannot consistently apply synchronization on objects across interpretation, executing in a single thread, and bytecode executing in one or more other threads.  We would need to update the interpreter to use native methods that invoke the JNI `MonitorEnter` and `MonitorExit` methods, although it is not clear if those can be mixed in a nested manner with JVM execution of monitor enter and exit bytecode instructions.
>> 
>> A simple test for generated bytecode has been included that tests for the expected number of monitor enter and exit instructions, and single threaded execution for non-exceptional and exceptional cases.
>
> src/java.base/share/classes/java/lang/reflect/code/interpreter/Interpreter.java line 567:
> 
>> 565:                     .map(String::valueOf)
>> 566:                     .collect(Collectors.joining());
>> 567:         } else if (o instanceof CoreOp.MonitorOp.MonitorEnterOp) {
> 
> Crazy suggestion: given the recent push of making more of the monitor logic be expressed in Java (as part of Loom), I wonder if it would make sense to imagine a world where a `synchronized` is just lowered to plain Java code - no special core ops? The really fundamental operation that cannot be expressed here is the "gimme a lock for this object". Something like:
> 
> 
> lock = lockFor(synchronizedValue) // (1)
> lock.lock();
> try {
>     // stats
> } finally {
>     lock.unlock();
> }
> 
> 
> Where (1) can be expressed as a core op similar to `ArrayLengthOp`.
> 
> I realize that what you have here is more 1-1 with the bytecode, hence, maybe preferrable. But I wanted to put it out there :-)

I would very much like to do that, alas we cannot today. `s.m.Unsafe` used to have methods to lock, try-lock, and unlock object monitors. I removed them :-) The JVM permits but does not enforce (HotSpot does enforce) constraints on structured locking (JVM spec section [2.11.10](https://docs.oracle.com/javase/specs/jvms/se22/html/jvms-2.html#jvms-2.11.10))

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

PR Review Comment: https://git.openjdk.org/babylon/pull/217#discussion_r1729169174