Hello Eric,
I have another one. Although not very likely, the reference to the same
Method/Constructor can be shared among multiple threads. The publication
of a parameters array should therefore be performed via a volatile write
/ volatile read, otherwise it can happen that some thread sees
half-initialized array content. The 'parameters' field in Executable
should be declared as volatile and there should be a single read from it
and a single write to it in the privateGetParameters() method (you need
a local variable to hold intermediate states)...
Regards, Peter
On 01/10/2013 09:42 PM, Eric McCorkle wrote:
Thanks to all for initial reviews; however, it appears that the version
you saw was somewhat stale. I've applied your comments (and some
changes that I'd made since the version that was posted).
Please take a second look.
Thanks,
Eric
On 01/10/13 04:19, Peter Levart wrote:
Hello Eric,
You must have missed my comment from the previous webrev:
292 private Parameter[] privateGetParameters() {
293 if (null != parameters)
294 return parameters.get();
If/when the 'parameters' SoftReference is cleared, the method will be
returning null forever after...
You should also retrieve the referent and check for it's presence before
returning it:
Parameter[] res;
if (parameters != null && (res = parameters.get()) != null)
return res;
...
...
Regards, Peter
On 01/09/2013 10:55 PM, Eric McCorkle wrote:
Hello,
Please review the core reflection API implementation of parameter
reflection. This is the final component of method parameter reflection.
This was posted for review before, then delayed until the check-in for
JDK-8004728 (hotspot support for parameter reflection), which occurred
yesterday.
Note: The check-in of JDK-8004728 was into hsx/hotspot-rt, *not*
jdk8/tl; therefore, it may be a while before the changeset makes its way
into jdk8/tl.
Also note: since the check-in of JDK-8004727 (javac support for
parameter reflection), there has been a failure in the tests for
Pack200. This is being addressed in a fix contributed by Kumar, which I
believe has also been posted for review.
The open webrev is here:
http://cr.openjdk.java.net/~coleenp/JDK-8004729
The feature request is here:
http://bugs.sun.com/view_bug.do?bug_id=8004729
The latest version of the spec can be found here:
http://cr.openjdk.java.net/~abuckley/8misc.pdf
Thanks,
Eric
