RFR: 8344197: SM cleanup in java.util.concurrent

[Roger Riggs]

On Fri, 15 Nov 2024 07:23:23 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Refactored to remove use of doPrivileged() and use of SecurityManager.
>> The DefaultForkJoinWorkerThreadFactory no longer uses the SM to target a common thread pool.
>> 
>> A careful review is requested.
>
> src/java.base/share/classes/java/util/concurrent/CopyOnWriteArrayList.java line 2104:
> 
>> 2102:             lockField.set(this, new Object());
>> 2103:         } catch (IllegalAccessException | NoSuchFieldException e) {
>> 2104:                 throw new RuntimeException(e);
> 
> Did you mean to change this from Error to RuntimeException? It shouldn't happen of course, but probably best not to change this.

Revert

> src/java.base/share/classes/java/util/concurrent/ForkJoinWorkerThread.java line 244:
> 
>> 242:         @Override // paranoically
>> 243:         public void setContextClassLoader(ClassLoader cl) {
>> 244:             // No-op, do not change the context class loader
> 
> I don't think this we should change this in this PR. Instead, I think we need to decide what to specify.

reverted

> src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java line 700:
> 
>> 698: 
>> 699:     private void interruptAll() {
>> 700:             implInterruptAll();
> 
> Rename implInterruptAll to InterruptAll and remove implInterruptAll.

will do

> src/java.base/share/classes/java/util/concurrent/atomic/AtomicIntegerFieldUpdater.java line 399:
> 
>> 397:                 if ((ccl != null) && (ccl != cl) &&
>> 398:                     ((cl == null) || !isAncestor(cl, ccl))) {
>> 399:                     sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
> 
> The ensureMemberAccess needs to stay but the checkPackageAccess can be removed.
> 
> @seanjmullan I think you'll want to study this one closely.

ok, removing package access check

> src/java.base/share/classes/java/util/concurrent/atomic/Striped64.java line 385:
> 
>> 383:         try {
>> 384:             MethodHandles.Lookup l2 =
>> 385:                  MethodHandles.privateLookupIn(Thread.class, MethodHandles.lookup());
> 
> I assume this can be changed to ` MethodHandles.Lookup l2 = MethodHandles.privateLookupIn(Thread.class, l1);`

ok

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/22119#discussion_r1844084122
PR Review Comment: https://git.openjdk.org/jdk/pull/22119#discussion_r1844084647
PR Review Comment: https://git.openjdk.org/jdk/pull/22119#discussion_r1844084807
PR Review Comment: https://git.openjdk.org/jdk/pull/22119#discussion_r1844085416
PR Review Comment: https://git.openjdk.org/jdk/pull/22119#discussion_r1844085740