search

ARM-software / synchronization-benchmarks

Collection of synchronization micro-benchmarks and traces from infrastructure applications
Other
37 stars 36 forks source link

Memory ordering in jvm_objectmonitor #58

Closed chacon01 closed 4 years ago

chacon01 commented 4 years ago

The release_store_thread function 

inline static void release_store_thread(volatile pthread_t* dest,
        pthread_t val) {
    READ_MEM_BARRIER;
    *dest = val;
    WRITE_MEM_BARRIER;
}

does not provide release semantics. When this is inlined into jvmObjectMonitorExitEpilog, it permits the store to omonitor->_succ to be visible after the store to omonitor->_owner, which violates what is expected in the JVM code.

lucasclucasdo commented 4 years ago

Agree. This could be fixed in several ways. What is the most appropriate option to match with likely JVM behavior across many platforms? __atomic_store_n(dest, val, __ATOMIC_RELEASE)?

chacon01 commented 4 years ago

Changing it to have

WRITE_MEM_BARRIER;
*dest = val;

would be sufficient, although would not necessarily be the most performant on architectures that have dedicated instructions to store with release semantics. I think __atomic_store_n(dest, val, __ATOMIC_RELEASE) is the right way to fix this.