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

[David Alvarez]

Hi,

The bug is marked as 9-na and 11-na, and there is a comment stating this
will be specific to 8u. Furthermore, Oracle has decided to patch this
same bug in 8u271, but not in tip.

David

On 2020-08-18 22:23, Andrew Hughes wrote:
> 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,
>