Thanks, Paul.
Looks good. Nits:
---
Our inline assignment style is non-standard, but we like it and it saves a
byte of bytecode occasionally, making things a tiny bit easier for the JIT.
---
The threadgroup creation code that uses cheating has always seemed fishy to
me. The straightforward
/**
* Returns a new group with the system ThreadGroup (the
* topmost, parent-less group) as parent.
*/
static final ThreadGroup createThreadGroup(String name) {
if (name == null)
throw new NullPointerException();
ThreadGroup group = Thread.currentThread().getThreadGroup();
for (ThreadGroup p; (p = group.getParent()) != null; )
group = p;
return new ThreadGroup(group, name);
}
passes all tests. That could be wrapped in a doPrivileged, or we could add
something more principled to ThreadGroup itself. I don't know what the
motivations are for the threadgroup code.
---
431 // Teleport the lookup to access fields in Thread
Teleport?! Maybe "Obtain a private lookup object" but that should be
obvious from the name "privateLookupIn"
On Wed, Jan 11, 2017 at 12:21 PM, Paul Sandoz <paul.san...@oracle.com>
wrote:
> Hi,
>
> Please review:
>
> http://cr.openjdk.java.net/~psandoz/jdk9/8160710-thread-to-tlr/webrev/
>
> This patch updates ThreadLocalRandom, Striped64 and LockSupport to use
> VarHandles instead of Unsafe to access fields in Thread, via the
> MethodsHandles.privateLookupIn method.
>
> Since there are three usages of MethodsHandles.privateLookupIn in
> ThreadLocalRandom i consolidated the doPrivileged block in a private static
> helper method, being paranoid i placed some checks to avoid this method
> being potentially abused to circumvent the security check, perhaps that is
> overkill?
>
> I ran this patch though our test infra a few times and have not observed
> any transient failures.
>
> Thanks,
> Paul.
>
