RFR: 8000975: (process) Merge UNIXProcess.java.bsd & UNIXProcess.java.linux (& .solaris & .aix)

[Martin Buchholz]

On Sun, May 4, 2014 at 3:26 AM, Peter Levart <peter.levart at gmail.com> wrote:

>
> Ah, I've forgotten to mention the most important change from webrev.07.
>
> Original code wraps the processReaperExecutor construction into the
> doPrivileged() call. I think this was not needed.
>
> The Executors.newCachedThreadPool() does not need any special privileges.
> And construction of nested class ProcessReaperThreadFactory also didn't
> need any special privileges - apart from static initialization which called
> the getRootThreadGroup() method. But that method did it's own
> doPrivileged() wrapping in order to be able to climb the ThreadGroup
> hierarchy (which requires privilege). In webrev.07 I followed original code
> and wrapped the construction of a processReaperExecutor into a
> doPrivileged() although in webrev.07 the rootThreadGroup search happens as
> part of UNIXProcess static initialization and is already wrapped in
> doPrivileged(). In webrev.08 I removed this superfluous doPrivileged(). I
> think this is correct. SecurityManagerClinit test passes.
>

Although I think you're right with the current implementation, it seems too
brittle to me to remove the doPrivileged, since you have no idea what
future changes to newCachedThreadPool() might do (or what other JDK
implementations might do); notably it might instantiate some kind of
micro-manager thread or do some other privileged operation.

If you want to have only one call to doPrivileged, you can cause both
rootThreadGroup and processReaperExecutor to be initialized in one static
block, although you will need to jump through some hoops to keep these
fields final.

---

As it stands,

Executors.newCachedThreadPool(grimReaper -> {

doesn't mention either the type ThreadFactory or Runnable.

It might be clearer (more verbosity in the spirit of Java) if you used
explicit ThreadFactory type

ThreadFactory factory = grimReaper -> ...