On 4/1/13 12:28 PM, Alan Bateman wrote:
On 27/03/2013 17:35, Mandy Chung wrote:
This is the JDK change for JEP 176: JEP 176: Mechanical Checking of Caller-Sensitive Methods [1]. Christian has posted the webrev for the hotspot VM change a couple weeks ago [2].
Webrev at: http://cr.openjdk.java.net/~mchung/jdk8/webrevs/7198429/webrev.00/
While it touches many files, the fix is simple and straight-forward for review. I went through the latest webrev.01 and this looks very good.
I guess I was initially surprised to see @CS on some of the AccessController.doPrivileged methods as these usages aren't checked (although they are caller sensitive).
These few methods are the special case that their usage are not checked. This raises a good point how we could enforce the check and whether it's appropriate to check in JVM_DoPrivileged. I will file a bug to follow up this separately if you are okay.
Did you consider adding a constant, JVM_DEPTH (-1) or some name like that, to jvm.h for the depth parameter?
Chris and I both agree it's a good thing to define this constant in jvm.h. I have made the change in the jdk side and will file a bug to update that in the hotspot repo.
I see Reflection.isCallerSensitive uses isExtClassLoader, we'll probably have to re-visit this in the future.
Yes.
Doug and Chris are on this list but might not have seen that this updates AtomicXXXXFieldUpdaters. They need to be aware of this for future merges.
On the tests, does GetCallerClassTest really need to check the stack trace? It seems unnecessary.
The stack trace check tries to catch if InternalError is thrown for other reason. Such regression might be rare but I prefer to perform an appropriate check while the test can reliably run.
One thing on the shell test (I read exchange about jtreg boot class path support) is that it needs the GPL header.
Fixed.
I was surprised to see CallerSensitiveFinder in the webrev and I'm curious how long it takes to run.
This is one discussion point I'd like to have. I was debating myself initially if this test should be enabled or not. What I found it takes 5-14 sec. Some sample timing on the jprt machines: linux_i586 jdk_lang took 14 mins and CallerSensitiveFinder took 8.5 sec macosx_x64 jdk_lang took 20.5 mins and CallerSensitiveFinder took 14.5 sec solaris_i586 jdk_lang took 15.5 mins and CallerSensitiveFinder took 10 sec windows_x64 jdk_lang took 16.5 mins and CallerSensitiveFinder took 10 sec We discussed tagging stress tests or long running tests so that they can be run on demand. I think this test would also be appropriate to be run in post-build hudson job, kind of tests. Mandy