Re: 答复: HotSpot and IBM's J9 behave quite differently when processing monitorenters and monitorexits

[David Holmes]

One correction ...

On 18/05/2017 10:29 AM, David Holmes wrote:
> On 18/05/2017 8:12 AM, 陈雨亭 wrote:
>> Thank you, David. I have seen from the specification that structured
>> locking
>> enforcement is optional. The second and the third ones are cases of
>> structured/nested lockings. Will non-nested locking sequences raise
>> deadlocks? Of course it is a different topic, while it might be better
>> if it
>> can be kicked out earlier from the specification/JVM.
>
> Non-nested locking doesn't necessarily lead to deadlocks - you just need
> a different locking order for that (even if properly nested). There are
> locking patterns that rely on the ability to lock and unlock in
> different order ie chained-locking for walking linked-lists A->B->C->D:
> - lock A, lock B, unlock A, lock C, unlock B, lock D, unlock C ...
>
> The VM spec allows a little flexibility in how the monitor bytecodes can
> be used compared to the Java programming language. That's not something
> that will change.
>
>> The first example is still a problem. It seems that HotSpot allows to
>> monitor a pure object reference without initialized (Is it true? How can
>> this checking be omitted?). J9 reports a verifyerror as follows.
>>
>> Exception in thread "main" java.lang.VerifyError: JVMVRFY012 stack shape
>> inconsistent; class=Search, method=main([Ljava/lang/String;)V, pc=6
>> Exception Details:
>>   Location:
>>     Search.main([Ljava/lang/String;)V @6: JBmonitorenter
>>   Reason:
>>     Type 'uninitialized' (current frame, stack[1]) is not assignable to
>> 'java/lang/Object'
>>   Current Frame:
>>     bci: @6
>>     flags: { }
>>     locals: { 'Search', '[Ljava/lang/String;' }
>>     stack: { 'uninitialized', 'uninitialized' }
>>     at T.main(T.java:4)
>
> Yes I think this may be a bug in hotspot. The type-checking for the
> monitor bytecodes requires a matching type of reference on the operand
> stack - but "uninitialized" does not match Object, as J9 reports. But
> I'm not an expert on this aspect of verification so I may not be
> interpreting it correctly.
>
> More below ...
>
>> -----邮件原件-----
>> 发件人: David Holmes [mailto:david.holmes at oracle.com]
>> 发送时间: 2017年5月17日 14:41
>> 收件人: 陈雨亭 <chenyt at cs.sjtu.edu.cn>;
>> hotspot-runtime-dev at openjdk.java.net
>> 主题: Re: HotSpot and IBM's J9 behave quite differently when processing
>> monitorenters and monitorexits
>>
>> Hi,
>> On 18/05/2017 7:23 AM, 陈雨亭 wrote:
>>> Am I wrong?
>>
>> I will look at each situation in detail when I get a chance but
>> structured
>> locking enforcement is optional. Also balancing the number of locks and
>> unlocks in a frame does not mean they can't be locked and unlocked in a
>> non-nested fashion - just that by the end the number of unlocks
>> matches the
>> number of locks.
>>
>> BTW the way you respond to these emails, as if having a conversation with
>> yourself, makes it difficult to respond as we can't readily see what
>> is the
>> new email and what is the original.
>>
>> Cheers,
>> David
>>
>>> The byte code for main() in case 1 is as follows. The strange thing is
>>> that NullPointerException is also not thrown at runtime.
>
> That is strange as it does for the normal obvious case of using
> synchronized(o) when o is null.

Ah - it isn't null it just an object for which the constructor has not 
been run. The runtime can't tell the difference between a pointer to a 
valid initialized object, and a pointer to an uninitialized chunk of memory.

>>>
>>> public void main(java.lang.String[]) throws java.lang.Exception;
>>>     descriptor: ([Ljava/lang/String;)V
>>>     flags: ACC_PUBLIC
>>>     Code:
>>>       stack=3, locals=2, args_size=2
>>>          0: new           #2                  // class Search

This allocated an object - hence no null reference. But this is what 
verification should have complained about.

David
-----

>>>          3: dup
>>>          4: aload_0
>>>          5: monitorenter
>>>          6: monitorenter
>>>          7: monitorexit
>>>          8: aload_0
>>>          9: monitorexit
>>>         10: return
>>>     Exceptions:
>>>       throws java.lang.Exception
>>>
>>> 主题: HotSpot and IBM's J9 behave quite differently when processing
>>> monitorenters and monitorexits
>>>
>>> I have tested several programs (in Jimple) and found that HotSpot and
>>> J9 match monitorenters and monitorexits quite differently. Verifiers
>>> should play more important roles here.
>
> The job of the verifier is to establish some basic guarantees for the
> JVM to then operate under. The verifier plays no role in checking how
> monitorenter/exit are used in combination, only that each individual
> bytecode meets some basic type constraints.
>
>>>
>>> (1) Test the next program (r2 is not initizlied) on HotSpot and J9.
>>> J9 throw out a verifier error, while HotSpot does not. It seems that
>>> HotSpot's verifier forgets to check whether a monitored object is
>>> initialized.
>>>
>>> public class Search extends java.lang.Object { public void <init>()
>>>     {
>>>         Search r0;
>>>         r0 := @this: Search;
>>>         specialinvoke r0.<java.lang.Object: void <init>()>();
>>>         return;
>>>     }
>>> public void main(java.lang.String[]) throws java.lang.Exception
>>>     {
>>>         Search r0;
>>>         Search r2;
>>>         java.lang.String[] r1;
>>>         r0 := @this: Search;
>>>         r1 := @parameter0: java.lang.String[];
>>>         r2 = new Search;
>>>
>>>         entermonitor r2;
>>>         entermonitor r0;
>>>         exitmonitor r2;
>>>         exitmonitor r0;
>>>         return;
>>>     }
>>> }
>
> Verification was covered above.
>
>>>
>>> (2) Test the next program on HotSpot and J9, and both do not report
>>> any errors. However, I guess the order in the program (entermonitor
>>> r2; => entermonitor r0; =>  exitmonitor r2; => exitmonitor r0;)
>>> violates the situation of "structured locking" (Structured locking is
>>> the situation when, during a method invocation, every exit on a given
>>> monitor matches a preceding entry on that monitor, see the
>>> specification
>>> https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-2.html#jvms-2.
>>> 11.10)
>>> ?
>
> No it doesn't violate structured locking as the number of enters and
> exits match, and there is always an enter before an exit.
>
>>> Actually, the words (every exit on a given monitor matches a preceding
>>> entry on that monitor) are not quite clear as for me. Otherwise the
>>> first rule (The number of monitor entries performed by T on M during a
>>> method invocation must equal the number of monitor exits performed by
>>> T on M during the method invocation whether the method  invocation
>>> completes normally or abruptly.) is sufficient.
>
> The number of enters and exits must not only match/balance, but there
> must be an enter before a corresponding exit.
>
>>>
>>> public class Search extends java.lang.Object {
>>>
>>> public void <init>()
>>>     {
>>>         Search r0;
>>>         r0 := @this: Search;
>>>         specialinvoke r0.<java.lang.Object: void <init>()>();
>>>         return;
>>>     }
>>>
>>> public void main(java.lang.String[]) throws java.lang.Exception
>>>     {
>>>         Search r0;
>>>         Search r2;
>>>         java.lang.String[] r1;
>>>         r0 := @this: Search;
>>>         r1 := @parameter0: java.lang.String[];
>>>         r2 = new Search;
>>>         specialinvoke r2.<Search: void <init>()>();
>>>         entermonitor r2;
>>>         entermonitor r0;
>>>         exitmonitor r2;
>>>         exitmonitor r0;
>>>         return;
>>>     }
>>> }
>>>
>>> (3) The next program enters monitor in <init> and exits it in main().
>>> HotSpot throws a runtime exception, while J9 does not. Should this
>>> program be rejected by the verifiers?
>
> No this does not violate any verification rules. The runtime behaviour
> depends on whether structured locking is enforced or not.(Even in
> hotspot there can be differences between interpreted and jitted code).
>
> Hope that helps clarify things.
>
> David
> -----
>
>>>
>>> public class Search extends java.lang.Object {
>>>
>>> public void <init>()
>>>     {
>>>         Search r0;
>>>         r0 := @this: Search;
>>>         specialinvoke r0.<java.lang.Object: void <init>()>();
>>>         entermonitor r0;
>>>         return;
>>>     }
>>>
>>> public void main(java.lang.String[]) throws java.lang.Exception
>>>     {
>>>         Search r0;
>>>         Search r2;
>>>         java.lang.String[] r1;
>>>         r0 := @this: Search;
>>>         r1 := @parameter0: java.lang.String[];
>>>         r2 = new Search;
>>>         specialinvoke r2.<Search: void <init>()>();
>>>         exitmonitor r2;
>>>         exitmonitor r0;
>>>         return;
>>>     }
>>> }
>>>
>>>
>>