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'.
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.
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 *
