java.net.http.ExecutorWrapper "memory fence"

[Michael McMahon]

Thanks Aleksey,

I will take care of it.

- Michael

On 09/03/16 11:34, Aleksey Shipilev wrote:
> 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
>>
>