Skip to content

Commit ed44c1a

Browse files
committed
CAtomic#get, #set, #swap have 'volatile' semantics
This patch ensures that a value #set on a CAtomic on one thread will immediately be visible through #get on another thread. The intention is that Atomic#get and #set should provide semantics like a Java 'volatile' variable. The whole point of storing a value in an Atomic is because it will be shared and modified from multiple threads, so it seems wrong that #get and #set provide no particular memory-visibility guarantees. This patch fixes that problem. It would be better to use a load barrier for #get and a store barrier for #set, but I don't know how to do that in a portable way. If maximum performance is desired, it might be necessary to use inline assembly, with #ifs based on the processor architecture. The other variants of Atomic should be patched to provide the same guarantees. The documentation should also be amended.
1 parent bc0d714 commit ed44c1a

File tree

2 files changed

+23
-3
lines changed

2 files changed

+23
-3
lines changed

ext/concurrent_ruby_ext/atomic_reference.c

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,31 @@ VALUE ir_initialize(int argc, VALUE* argv, VALUE self) {
2727
}
2828

2929
VALUE ir_get(VALUE self) {
30+
#if HAVE_GCC_SYNC
31+
__sync_synchronize();
32+
#elif defined _MSC_VER
33+
MemoryBarrier();
34+
#elif __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1050
35+
OSMemoryBarrier();
36+
#endif
3037
return (VALUE) DATA_PTR(self);
3138
}
3239

3340
VALUE ir_set(VALUE self, VALUE new_value) {
3441
DATA_PTR(self) = (void *) new_value;
42+
#if HAVE_GCC_SYNC
43+
__sync_synchronize();
44+
#elif defined _MSC_VER
45+
MemoryBarrier();
46+
#elif __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1050
47+
OSMemoryBarrier();
48+
#endif
3549
return new_value;
3650
}
3751

3852
VALUE ir_get_and_set(VALUE self, VALUE new_value) {
39-
VALUE old_value;
40-
old_value = (VALUE) DATA_PTR(self);
41-
DATA_PTR(self) = (void *) new_value;
53+
VALUE old_value = ir_get(self);
54+
ir_set(self, new_value);
4255
return old_value;
4356
}
4457

ext/concurrent_ruby_ext/extconf.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,13 @@ def compiler_is_gcc
4343
end
4444
end
4545

46+
try_run(<<CODE,$CFLAGS) && ($defs << '-DHAVE_GCC_SYNC')
47+
int main() {
48+
__sync_synchronize();
49+
return 0;
50+
}
51+
CODE
52+
4653
create_makefile(EXTENSION_NAME)
4754
rescue
4855
create_dummy_makefile

0 commit comments

Comments
 (0)