[8u] RFR 8249846: Change of behavior after JDK-8237117: Better ForkJoinPool behavior

Andrew Hughes gnu.andrew at redhat.com
Wed Aug 19 05:23:50 UTC 2020


On 12:10 Tue 18 Aug     , David Alvarez wrote:
> Hi,
> 
> I would like to request a review for JDK-8249846
> 
> During the backport of JDK-8237117 a regression was added that affected
> how the DefaultForkJoinWorkerThreadFactory works. The idea was to change
> the context for threads in the commonPool, but it ended up affecting the
> threads in all pools created with that factory. This patch decouples the
> commonPool from the DefaultForkJoinWorkerThreadFactory
> 
>   Bug: https://bugs.openjdk.java.net/browse/JDK-8249846
>   Webrev:
> http://cr.openjdk.java.net/~alvdavi/webrevs/8249846/webrev.8u.jdk.00/
> 
> My original intention when writing the patch was to make the CommonPool
> use the InnocuousForkJoinWorkerThreadFactory [1] in all cases,
> regardless of whether a SecurityManager is installed or not, as we are
> already creating threads with an innocuous AccessControlContext for
> commonPool threads even when there is no SecurityManager installed.
> However, the InnocuousForkJoinWorkerThreadFactory creates
> InnocuousForkJoinWorkerThreads [2], which have some more nuances, like
> the erase of ThreadLocal variables or avoid setting an
> UncaughtExceptionHandler. I was afraid of introducing more regressions
> with this change (in fact, one jtreg test [3] did fail because it was
> setting an uncaught exception handler), that is why I decided to go for
> a new factory (DefaultCommonPoolForkJoinWorkerThreadFactory) that mimics
> current behavior, even if it means the behavior of the common pool if a
> SecurityManager is installed during initialization is still not the same
> as if the SecurityManager is installed after initialization.
> 
> 
> David
> 
> --
> [1]
> http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk/file/4d586f39fed3/src/share/classes/java/util/concurrent/ForkJoinPool.java#l3429
> [2]
> http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk/file/4d586f39fed3/src/share/classes/java/util/concurrent/ForkJoinWorkerThread.java#l231
> [3]
> http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk/file/4d586f39fed3/test/java/util/concurrent/forkjoin/ThrowingRunnable.java#l66
> 

Should this not go to jdk/jdk first or is it only applicable to 8u?

Thanks,
-- 
Andrew :)

Senior Free Java Software Engineer
OpenJDK Package Owner
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222


More information about the jdk8u-dev mailing list