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@. 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
