On Fri, 15 Nov 2024 07:23:23 GMT, Alan Bateman <al...@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
