The worst thing here is that conceptually, for an invokedynamic, the
implementation do not use the fact that MethodTypes are interned.
Only a direct call to a method handle (with invokeExact/invoke) needs
MethodTypes to be interned to speedup the test that verifies that the parameter
types are compatible.
One way to solve that is to not use the cache when creating a MethodType but
only when invoking a method handle directly. A MethodHandle can have two
fields, one storing the non-interned method type (which is final) and one
storing the corresponding interned method type (which is @Stable) and computed
it if the pointer check of an invokeExact check fail because
mh.invokeExact(desc)
if (mh.internedMethodType == desc.methodType) {
// fastpath
}
if (mh.internedMethodType == null) {
mh.internedMethodType = intern(mh.type);
if (mh.internedMethodType == desc.methodType) {
// fastpath
}
}
...
Rémi
----- Mail original -----
> De: "David Holmes" <david.hol...@oracle.com>
> À: "Aleksey Shipilev" <sh...@redhat.com>, "core-libs-dev Libs"
> <core-libs-dev@openjdk.java.net>
> Envoyé: Mercredi 1 Mars 2017 13:36:49
> Objet: Re: Forcing initialization of string concat INDY expressions
> 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@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@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
