java.net.http.ExecutorWrapper "memory fence"

[Aleksey Shipilev]

Alan mentioned I should have sent this to net-dev at . Instead, I submitted
a new bug:
 https://bugs.openjdk.java.net/browse/JDK-8151505

-Aleksey

On 03/09/2016 02:06 PM, Aleksey Shipilev wrote:
> Hi,
> 
> In recently committed java.net.http.ExecutorWrapper, there is a
> synchronize() method [1], which is used as "memory fence" [2]:
> 
>  public synchronized void synchronize() {}
> 
>  public void execute(Runnable r, Supplier<AccessControlContext>
> ctxSupplier) {
>    synchronize();
>    Runnable r1 = () -> {
>      try {
>        r.run();
>      } catch (Throwable t) {
>        Log.logError(t);
>      }
>    };
> 
>    ...
> 
>    executor.execute(r1);
>  }
> 
> 
> How's that supposed to work? Is that supposed to guard from bad Runnable
> $r?
> 
> The problem is, once you get $r via the race, there is no way to recover
> with local synchronization (IOW: There is no way to sanitize a racy
> input, once it happened. Races are bad like that) And if $r got to you
> properly, you don't need to do anything special too (IOW: API may as
> well assume it is coming from the current thread).
> 
> Therefore, I think synchronize() method there is superfluous.
> 
> In fact, assuming that a synchronized method has any *detached* memory
> semantics is wrong too -- compilers are known to elide associated
> fences. E.g. if ExecutorWrapper is known to never escape a thread, or a
> single thread locks on it, and biases a lock towards itself.
> 
> Thanks,
> -Aleksey
> 
> [1]
> http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/e0da6c2a5c32/src/java.httpclient/share/classes/java/net/http/ExecutorWrapper.java#l74
> [2]
> http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/e0da6c2a5c32/src/java.httpclient/share/classes/java/net/http/ExecutorWrapper.java#l77
>