Skip to content

Commit 11d4648

Browse files
committed
Add required STORE_STORE barrier for shared object writes
* Particularly important for architectures where writes can reorder. * The compiler did not move the write in the synchronized block in my testing, but I could observe hardware write reordering on ARM64 when removing the synchronized block. Better be safe than sorry for thread safety. * Thanks to Professor Beverly A. Sanders for first bringing this to my attention.
1 parent 5f5d8a6 commit 11d4648

File tree

1 file changed

+9
-0
lines changed

1 file changed

+9
-0
lines changed

src/main/java/org/truffleruby/language/objects/WriteObjectFieldNode.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.oracle.truffle.api.profiles.BranchProfile;
2626
import com.oracle.truffle.api.utilities.NeverValidAssumption;
2727

28+
import org.truffleruby.extra.ffi.Pointer;
2829
import org.truffleruby.language.RubyBaseNode;
2930
import org.truffleruby.language.RubyGuards;
3031
import org.truffleruby.language.objects.shared.SharedObjects;
@@ -72,6 +73,14 @@ public void writeExistingField(DynamicObject object, Object value, boolean gener
7273
try {
7374
if (shared) {
7475
writeBarrierNode.executeWriteBarrier(value);
76+
/*
77+
* We need a STORE_STORE memory barrier here, to ensure the value is seen as shared by all threads
78+
* when published below by writing the value to a field of the object.
79+
* Otherwise, the compiler could theoretically move the write barrier
80+
* inside the synchronized block, and then the compiler or hardware could potentially
81+
* reorder the writes so that publication would happen before sharing.
82+
*/
83+
Pointer.UNSAFE.storeFence();
7584
synchronized (object) {
7685
// Re-check the shape under the monitor as another thread might have changed it
7786
// by adding a field (fine) or upgrading an existing field to Object storage

0 commit comments

Comments
 (0)