[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR: 8038468: java/lang/instrument/ParallelTransformerLoader.sh fails with ClassCircularityError
From: David Holmes <david.holmes () oracle ! com>
Date: 2014-10-22 3:12:52
Message-ID: 544720B4.6080102 () oracle ! com
[Download RAW message or body]
On 22/10/2014 12:52 PM, Yumin Qi wrote:
> Hi, David and all,
>
> Second webrev here: http://cr.openjdk.java.net/~minqi/8038468/webrev01/
>
> Answer to David's question about 'main' and 'DestroyJavaVM'. I still
> did not find how when exception printing the stack trace, 'main' was
> retrieved but, at the moment JavaThread for "DestroyJavaVM' was created,
> 'main' is not dead. They may exist and with same C thread and id. This
> may cause we got 'main' not 'DestroyJavaVM'.
They are the same native thread. The "main" thread, upon return from
main() (the Java app main()) detaches from the VM and re-attaches as the
DestroyJavaVMThread. So I don't see how an exception dump can show the
thread name as "main".
> Loading another class from agent in 'transform' with same custom
> class loader is not a good design. We already have two threads loading
> from the agent in parallel, TestClass1 in 'main' and TestClass2 in
> 'TestThread'. Should avoid loading another class with same agent in
> 'transform' in nested.
I'm really not sure we're getting to the bottom of this, but I don't
understand the original form and purpose of the test.
David
>
> Thanks
> Yumin
>
> On 10/17/2014 10:54 PM, David Holmes wrote:
>> Hi Yumin,
>>
>> Quick response ... when shutdown is initiated the Shutdown class will
>> be loaded and initialized:
>>
>> at java.lang.Shutdown.<clinit>(Shutdown.java:61)
>>
>> Presumably this static initialization is what triggers the involvement
>> of the agent to do the transform, and hence encounters the exception.
>> Though I'm unclear how it still reports "main" as the name when it has
>> now become "DestroyJavaVM"
>>
>> David
>>
>> On 18/10/2014 3:08 PM, Yumin Qi wrote:
>>> David, (cc Karen)
>>>
>>> I think I got why it throws CircularityError in 'main' thread.
>>> The CircularityError thrown in TestThread, which was handled in
>>> classloading, the loading class is put into unresolved list. Note we
>>> clean pending exception and return null to caller, which in the search
>>> next will load the instance class. There is no exception in java level
>>> be caught in TestThread.
>>> When main ended, we create a JavaThread named 'DestroyJavaVM' and
>>> give the thread id the current thread id, which is the main thread id.
>>> Since the All JavaThread object should be freed when this last
>>> JavaThread exit, I have no idea how the 'DestroyJavaVM' thread saw the
>>> exception, from the stack trace, the calling begins with
>>>
>>> ShutDown.java:
>>>
>>> /* The preceding static fields are protected by this lock */
>>> private static class Lock { };
>>> private static Object lock = new Lock();
>>> //<<<------------------------ line 61
>>>
>>> How come the call via agent and call transform? At shutdown time, do
>>> we need to turn down the request to agent at this time?
>>>
>>> Thanks
>>> Yumin
>>>
>>>
>>> java.lang.Exception: Stack trace
>>> at java.lang.Thread.dumpStack(Thread.java:1329)
>>> at
>>> ParallelTransformerLoaderAgent$TestTransformer.transform(ParallelTransformerLoaderAgent.java:92)
>>>
>>>
>>> at
>>> sun.instrument.TransformerManager.transform(TransformerManager.java:188)
>>> at
>>> sun.instrument.InstrumentationImpl.transform(InstrumentationImpl.java:428)
>>>
>>> java.lang.Exception: Stack trace
>>> at java.lang.Thread.dumpStack(Thread.java:1329)
>>> at
>>> ParallelTransformerLoaderAgent$TestTransformer.transform(ParallelTransformerLoaderAgent.java:92)
>>>
>>>
>>> at
>>> sun.instrument.TransformerManager.transform(TransformerManager.java:188)
>>> at
>>> sun.instrument.InstrumentationImpl.transform(InstrumentationImpl.java:428)
>>>
>>> at java.lang.Shutdown.<clinit>(Shutdown.java:61)
>>>
>>> This output in
>>>
>>>
>>>
>>>
>>>
>>> On 10/15/2014 9:58 AM, Yumin Qi wrote:
>>>> David,
>>>>
>>>> I will take another detail trace to see where the exception begins
>>>> in main thread, it should not thrown in main thread. I only saw it is
>>>> thrown in TestThread, not main, not DestroyJavaVM. If that happens,
>>>> maybe something wrong in vm.
>>>> The output in all 'failed' case (many failed not cause exception
>>>> output, not caught), the main thread got the exception. That is not
>>>> right.
>>>>
>>>> Thanks
>>>> Yumin
>>>>
>>>> On 10/14/2014 5:28 PM, David Holmes wrote:
>>>>> Hi Yumin,
>>>>>
>>>>> On 15/10/2014 4:40 AM, Yumin Qi wrote:
>>>>>> David, Thanks for the comment. See embedded.
>>>>>>
>>>>>>
>>>>>> On 10/13/2014 7:30 PM, David Holmes wrote:
>>>>>>> Hi Yumin,
>>>>>>>
>>>>>>> jdk9-dev is not the best place for code review requests.
>>>>>>> serviceability-dev would be better for this test.
>>>>>>>
>>>>>>> On 14/10/2014 8:58 AM, Yumin Qi wrote:
>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8038468
>>>>>>>> webrev:*http://cr.openjdk.java.net/~minqi/8038468/webrev00/
>>>>>>>>
>>>>>>>> the bug marked as confidential so post the webrev internally.
>>>>>>>
>>>>>>> Not any more :)
>>>>>>>
>>>>>> Thanks. I changed to non security related bug. Usually when test
>>>>>> failed,
>>>>>> a confidential bug is filed. I would like to create bug open if the
>>>>>> test
>>>>>> is in open part.
>>>>>>>> Problem: The test case tries to load a class from the same jar via
>>>>>>>> agent
>>>>>>>> in the middle of loading another class from the jar via same class
>>>>>>>> loader in same thread. The call happens in transform which is a
>>>>>>>> rare
>>>>>>>> case --- in middle of loading class, loading another class. The
>>>>>>>> result
>>>>>>>> is a CircularityError. When first class is in loading, in vm we put
>>>>>>>> JarLoader$2 on place holder table, then we start the defineClass,
>>>>>>>> which
>>>>>>>> calls transform, begins loading the second class so go along the
>>>>>>>> same
>>>>>>>> routine for loading JarLoader$2 first, found it already in
>>>>>>>> placeholder
>>>>>>>> table. A CircularityError is thrown.
>>>>>>>> Fix: The test case should not call loading class with same class
>>>>>>>> loader
>>>>>>>> in same thread from same jar in 'transform' method. I modify it
>>>>>>>> loading
>>>>>>>> with system class loader and we expect see ClassNotFoundException.
>>>>>>>> Detail see bug comments.
>>>>>>>
>>>>>>> It is not clear to me that the test is incorrect. It is also unclear
>>>>>>> why such an old test is now failing - we must have changed
>>>>>>> something.
>>>>>>> And it's unclear whether what the test does with your change is
>>>>>>> actually testing what the test wanted to test.
>>>>>>>
>>>>>>> It seems to me that the actual problem in the test is the
>>>>>>> reference to
>>>>>>> the "main" thread ie:
>>>>>>>
>>>>>>> if (!tName.equals("main"))
>>>>>>>
>>>>>>> The test knows not to do the loading in the main thread, but has
>>>>>>> overlooked the fact that the main thread, upon the end of main()
>>>>>>> becomes the DestroyJavaVM thread - and it is that thread which
>>>>>>> encounters the ClassCircularityError:
>>>>>>>
>>>>>>> Starting test with 1000 iterations
>>>>>>> Thread 'DestroyJavaVM' has called transform()
>>>>>>>
>>>>>>> So perhaps the right fix is to expand the above to:
>>>>>>>
>>>>>>> if (!tName.equals("main") && !tName.equals("DestroyJavaVM"))
>>>>>>>
>>>>>>> ? I admit I'm having trouble seeing the full picture in this test.
>>>>>>>
>>>>>> It is not DestroyJavaVM thread cause CircularityError. It is
>>>>>> TestThread
>>>>>> cause CircularityError.
>>>>>
>>>>> Not according to the bug report:
>>>>>
>>>>> Starting test with 1000 iterationsThread 'DestroyJavaVM' has called
>>>>> transform()
>>>>> Thread 'DestroyJavaVM' has called transform()
>>>>> result=1
>>>>> ----------System.err:(14/920)----------
>>>>> Exception in thread "main" java.lang.ClassCircularityError:
>>>>> sun/misc/URLClassPath$JarLoader$2
>>>>>
>>>>> This shows that "main" got the CCE. Which in itself is confusing
>>>>> given we also report "Thread 'DestroyJavaVM' has called transform()"
>>>>> and they are in fact the same thread!
>>>>>
>>>>> David
>>>>> -----
>>>>>
>>>>>
>>>>>> In TestThread (DestroyJavaVM may cause same I think, but not seen in
>>>>>> debug):
>>>>>>
>>>>>> forName("TestClass2", true, classLoader); <---- the loader is
>>>>>> customer loader which is obtained from agent code.
>>>>>> -->...... transform(...)
>>>>>> -->defineClass(...)
>>>>>> -->...... call into vm, we need to load
>>>>>> JarLoader$2
>>>>>> since JarLoader$1 used
>>>>>> ->resolve_instance_class_or_null
>>>>>> // here we create PlaceTableEntry for
>>>>>> JarLoader$2, put into place holder table
>>>>>> -->......
>>>>>> --->forName("TestClass3", true,
>>>>>> classLoader);
>>>>>> -->... transform(...)
>>>>>> -->defineClass(...)
>>>>>> -->...... call into vm
>>>>>> again. Now JarLoader$2 is not loaded, but it is in placeholder
>>>>>> table, so
>>>>>> throw_circularity_error set and throw.
>>>>>> .......
>>>>>> With custom loader, agent's transform will be called, then it
>>>>>> loads TestClass3, repeat the same steps as loading TestClass2. The
>>>>>> problem is JarLoader$2 has not been loaded yet but in place holder
>>>>>> table
>>>>>> (this is for checking CircularityError), then begins loading
>>>>>> TestClass3,
>>>>>> this is a recursive and embedded case. The non-failed case also saw
>>>>>> CircularityError thrown, but somehow the test case did not fail.
>>>>>> Design
>>>>>> like this will cause call transform in transform which is the reason
>>>>>> CircularityError thrown.
>>>>>>
>>>>>> I have no idea about the original desin of the test case, but
>>>>>> think
>>>>>> it should do this.
>>>>>>
>>>>>>>
>>>>>>> Looking at your change, don't leave commented out lines in the code:
>>>>>>> 115 // ClassLoader loader =
>>>>>>> ParallelTransformerLoaderAgent.getClassLoader();
>>>>>>> 118 //Class.forName("TestClass" +
>>>>>>> index, true, loader);
>>>>>>>
>>>>>> Will remove
>>>>>>
>>>>>> Thanks
>>>>>> Yumin
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>>> Thanks
>>>>>>>> Yumin *
>>>>>>
>>>>
>>>
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic