RFR 8160710: Enable Thread to grant VarHandle field access to ThreadLocalRandom/Striped64

Martin Buchholz martinrb at google.com
Wed Jan 11 22:00:12 UTC 2017


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.sandoz at 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.
>


More information about the core-libs-dev mailing list