Looks good! On Wed, Jan 11, 2017 at 4:48 PM, Paul Sandoz <paul.san...@oracle.com> wrote:
> Thanks. > > Here is an update: > > http://cr.openjdk.java.net/~psandoz/jdk9/8160710-thread-to-tlr/webrev/ > > With changes to inline expressions, remove thread group stuff from TLR, > and do the following in InnocuousForkJoinWorkerThread: > > 185 static final class InnocuousForkJoinWorkerThread extends > ForkJoinWorkerThread { > 186 /** The ThreadGroup for all InnocuousForkJoinWorkerThreads */ > 187 private static final ThreadGroup innocuousThreadGroup = > 188 java.security.AccessController.doPrivileged( > 189 new java.security.PrivilegedAction<>() { > 190 public ThreadGroup run() { > 191 ThreadGroup group = > Thread.currentThread().getThreadGroup(); > 192 for (ThreadGroup p; (p = > group.getParent()) != null; ) > 193 group = p; > 194 return new ThreadGroup(group, " > InnocuousForkJoinWorkerThreadGroup"); > 195 }}); > > Note: the ThreadGroup constructor also performs a security permission check > > Paul. > > > On 11 Jan 2017, at 16:12, Doug Lea <d...@cs.oswego.edu> wrote: > > > > On 01/11/2017 06:39 PM, Martin Buchholz wrote: > >> Doug, please try to remember the details of the thread group code. > >> > > > >> > > >> > 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. > >> > > >> > > > > There was a some point a problem with using doPrivileged in the > > context of FJP.commonPool initialization, so we went with the > > suggestion of using unsafe. > > > > And at a different point, we decided to place > > all non-VarHandle-amenable code into TLR so that it could be > > replaced all at once when bootstrap problems were addressed. > > > > And at a different point (which we hope is Now :-) we can > > replace the unnecessary prior workarounds. > > > > -Doug > > > > > > > >