Forcing initialization of string concat INDY expressions

[David Holmes]

Hi Aleksey,

On 1/03/2017 7:46 PM, Aleksey Shipilev wrote:
> Hi,
>
> On 03/01/2017 07:36 AM, David Holmes wrote:
>> The INDY-fication of string concatenation has triggered a problem where a JVM TI
>> agent's monitor-wait/ed callback hits an error path that uses string concat
>> which triggers a mass of indy related initialization, which in turn hits monitor
>> use in MethodType$ConcurrentWeakInternSet.get, which causes the VM monitor
>> subsystem to be re-entered (and it is not reentrant!) so we crash. (log extract
>> below - the amount of code to process this is truly scary!)
>
> Ouch. This is an unexpected circularity. It is unusual to see Java thread to do
> Java stuff when doing Object.wait.

That's the fatal flaw in JVM TI. It pretends you can invoke arbitrary 
Java code via an agent callback. You can invoke simple Java code - as 
most real callbacks do - but not arbitrary Java code. The VM is not 
reentrant in some subsystems - like monitors.

>> I assume I can cover the exact case above by replicating it? But can I
>> generalize it to cover arbitrary string concat expressions that might arise on
>> other error paths?
>
> Yes, the StringConcatFactory code is deliberately lazy. If you want to link
> eagerly, you would need to match the concat shape exactly (number and type of
> arguments) -- probably by using the same concat code the test failed on. We can
> probably eagerly link some popular concat shapes early during system init, but
> that requires JDK changes.

So I can cover the current failing case easily enough but can't 
generalize except by covering each case individually.

> Looking at stack trace, it seems to be generic failure when adding new
> MethodType to the MethodType cache. This is not the first time that cache
> participates in circularities (I wonder if it *always* does). But I am puzzled
> how does this happen:
>
> ...
> V  [jvm.dll+0x2b5412]  Runtime1::monitorenter+0x1e2;;
> ?monitorenter at Runtime1@@CAXPAVJavaThread@@PAVoopDesc@@PAVBasicObjectLock@@@Z+0x1e2
> v  ~RuntimeStub::monitorenter_nofpu Runtime1 stub
> J 192 c1
> java.lang.invoke.MethodType$ConcurrentWeakInternSet.get(Ljava/lang/Object;)Ljava/lang/Object;
> java.base at 9-internal (54 bytes) @ 0x0242d7da [0x0242d4c0+0x0000031a]
> ...
>
> It calls RuntimeStub::monitorenter_nofpu destructor? Why it ends up calling into
> monitorenter? There are no locks in j.l.i.MT$CWIS.get (I checked the bytecode too).

get() is compiled so I'm assuming it has inlined something from CHM that 
does the locking.

> If you want a nuclear option for your test, you may want to pass
> -XDstringConcat:inline to javac to disable indy string concat to bypass that
> circularity completely.

Hmmmm ... that is worth thinking about.

Thanks,
David

> Thanks,
> -Aleksey
>