RFR: JDK-8286666: JEP 429: Implementation of Scoped Values (Incubator)

Andrew Haley aph at openjdk.org
Tue Nov 15 14:20:53 UTC 2022


On Thu, 3 Nov 2022 11:50:17 GMT, ExE Boss <duke at openjdk.org> wrote:

>> JEP 429 implementation.
>
> src/java.base/share/classes/java/lang/Thread.java line 1610:
> 
>> 1608:             ensureMaterializedForStackWalk(bindings);
>> 1609:             task.run();
>> 1610:             Reference.reachabilityFence(bindings);
> 
> This should probably be in a `try`‑`finally` block:
> Suggestion:
> 
>             try {
>                 task.run();
>             } finally {
>                 Reference.reachabilityFence(bindings);
>             }

I wonder. The pattern I'm using here is based on `AccessController.executePrivileged`, which doesn't have the `finally` clause. Perhaps I should add one here anyway.

> src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/ScopedValue.java line 481:
> 
>> 479:         }
>> 480:  */
>> 481:         return findBinding() != Snapshot.NIL;
> 
> This should probably call `Cache.put(this, value)` when `findBinding()` isn’t `Snapshot.NIL`, since it’s likely that `isBound()` will most commonly be used in the form of:
> 
> if (SCOPED_VALUE.isBound()) {
> 	final var value = SCOPED_VALUE.get();
> 	// do something with `value`
> }
> 
> 
> --------------------------------------------------------------------------------
> 
> Suggestion:
> 
>         var value = findBinding();
>         if (value == Snapshot.NIL) {
>             return false;
>         }
>         Cache.put(this, value);
>         return true;

Probably so, yes. I'll have a look at that along with caching failure.

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

PR: https://git.openjdk.org/jdk/pull/10952


More information about the security-dev mailing list