Skip to content

Commit 0bd741c

Browse files
committed
Fix Synchronization on JRuby
1 parent bfee210 commit 0bd741c

File tree

7 files changed

+122
-77
lines changed

7 files changed

+122
-77
lines changed
Lines changed: 115 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package com.concurrent_ruby.ext;
22

33
import java.io.IOException;
4-
import java.util.concurrent.atomic.AtomicBoolean;
54

65
import org.jruby.Ruby;
76
import org.jruby.RubyClass;
@@ -14,42 +13,70 @@
1413
import org.jruby.runtime.load.Library;
1514
import org.jruby.runtime.Block;
1615
import org.jruby.runtime.Visibility;
17-
import org.jruby.RubyBoolean;
18-
import org.jruby.RubyNil;
1916
import org.jruby.runtime.ThreadContext;
2017
import org.jruby.util.unsafe.UnsafeHolder;
2118

2219
public class SynchronizationLibrary implements Library {
2320

24-
private static final ObjectAllocator JRUBYREFERENCE_ALLOCATOR = new ObjectAllocator() {
21+
private static final ObjectAllocator JRUBY_OBJECT_ALLOCATOR = new ObjectAllocator() {
2522
public IRubyObject allocate(Ruby runtime, RubyClass klazz) {
26-
return new JavaObject(runtime, klazz);
23+
return new JRubyObject(runtime, klazz);
24+
}
25+
};
26+
27+
private static final ObjectAllocator OBJECT_ALLOCATOR = new ObjectAllocator() {
28+
public IRubyObject allocate(Ruby runtime, RubyClass klazz) {
29+
return new Object(runtime, klazz);
30+
}
31+
};
32+
33+
private static final ObjectAllocator ABSTRACT_LOCKABLE_OBJECT_ALLOCATOR = new ObjectAllocator() {
34+
public IRubyObject allocate(Ruby runtime, RubyClass klazz) {
35+
return new AbstractLockableObject(runtime, klazz);
36+
}
37+
};
38+
39+
private static final ObjectAllocator JRUBY_LOCKABLE_OBJECT_ALLOCATOR = new ObjectAllocator() {
40+
public IRubyObject allocate(Ruby runtime, RubyClass klazz) {
41+
return new JRubyLockableObject(runtime, klazz);
2742
}
2843
};
2944

3045
public void load(Ruby runtime, boolean wrap) throws IOException {
3146
RubyModule synchronizationModule = runtime.
3247
defineModule("Concurrent").
3348
defineModuleUnder("Synchronization");
34-
RubyClass parentClass = synchronizationModule.getClass("AbstractObject");
3549

36-
if (parentClass == null)
37-
throw runtime.newRuntimeError("Concurrent::Synchronization::AbstractObject is missing");
50+
defineClass(runtime, synchronizationModule, "AbstractObject", "JRubyObject",
51+
JRubyObject.class, JRUBY_OBJECT_ALLOCATOR);
3852

39-
RubyClass synchronizedObjectJavaClass =
40-
synchronizationModule.defineClassUnder("JavaObject", parentClass, JRUBYREFERENCE_ALLOCATOR);
53+
defineClass(runtime, synchronizationModule, "JRubyObject", "Object",
54+
Object.class, OBJECT_ALLOCATOR);
4155

42-
synchronizedObjectJavaClass.defineAnnotatedMethods(JavaObject.class);
56+
defineClass(runtime, synchronizationModule, "Object", "AbstractLockableObject",
57+
AbstractLockableObject.class, ABSTRACT_LOCKABLE_OBJECT_ALLOCATOR);
58+
59+
defineClass(runtime, synchronizationModule, "AbstractLockableObject", "JRubyLockableObject",
60+
JRubyLockableObject.class, JRUBY_LOCKABLE_OBJECT_ALLOCATOR);
4361
}
4462

45-
@JRubyClass(name = "JavaObject", parent = "AbstractObject")
46-
public static class JavaObject extends RubyObject {
63+
private RubyClass defineClass(Ruby runtime, RubyModule namespace, String parentName, String name,
64+
Class javaImplementation, ObjectAllocator allocator) {
65+
final RubyClass parentClass = namespace.getClass(parentName);
4766

48-
public static final long AN_VOLATILE_FIELD_OFFSET =
49-
UnsafeHolder.fieldOffset(JavaObject.class, "anVolatileField");
50-
private volatile int anVolatileField = 0;
67+
if (parentClass == null) {
68+
System.out.println("not found " + parentName);
69+
throw runtime.newRuntimeError(namespace.toString() + "::" + parentName + " is missing");
70+
}
5171

52-
public JavaObject(Ruby runtime, RubyClass metaClass) {
72+
final RubyClass newClass = namespace.defineClassUnder(name, parentClass, allocator);
73+
newClass.defineAnnotatedMethods(javaImplementation);
74+
return newClass;
75+
}
76+
77+
@JRubyClass(name = "JRubyObject", parent = "AbstractObject")
78+
public static class JRubyObject extends RubyObject {
79+
public JRubyObject(Ruby runtime, RubyClass metaClass) {
5380
super(runtime, metaClass);
5481
}
5582

@@ -58,6 +85,77 @@ public IRubyObject initialize(ThreadContext context) {
5885
return this;
5986
}
6087

88+
@JRubyMethod(name = "full_memory_barrier", visibility = Visibility.PRIVATE)
89+
public IRubyObject fullMemoryBarrier(ThreadContext context) {
90+
if (UnsafeHolder.U == null) {
91+
// We are screwed
92+
throw new UnsupportedOperationException();
93+
} else if (UnsafeHolder.SUPPORTS_FENCES)
94+
UnsafeHolder.fullFence();
95+
else {
96+
// TODO (pitr 06-Sep-2015): enforce Java 8
97+
throw new UnsupportedOperationException();
98+
}
99+
return context.nil;
100+
}
101+
102+
@JRubyMethod(name = "instance_variable_get_volatile", visibility = Visibility.PROTECTED)
103+
public IRubyObject instanceVariableGetVolatile(ThreadContext context, IRubyObject name) {
104+
if (UnsafeHolder.U == null) {
105+
synchronized (this) {
106+
// TODO (pitr 06-Sep-2015): Possibly dangerous, there may be a deadlock here
107+
// TODO (pitr 08-Sep-2015): maybe remove the branch since full_memory_barrier is not supported anyway
108+
return instance_variable_get(context, name);
109+
}
110+
} else if (UnsafeHolder.SUPPORTS_FENCES) {
111+
// ensure we see latest value
112+
UnsafeHolder.loadFence();
113+
return instance_variable_get(context, name);
114+
} else {
115+
throw new UnsupportedOperationException();
116+
}
117+
}
118+
119+
@JRubyMethod(name = "instance_variable_set_volatile", visibility = Visibility.PROTECTED)
120+
public IRubyObject InstanceVariableSetVolatile(ThreadContext context, IRubyObject name, IRubyObject value) {
121+
if (UnsafeHolder.U == null) {
122+
synchronized (this) {
123+
return instance_variable_set(name, value);
124+
}
125+
} else if (UnsafeHolder.SUPPORTS_FENCES) {
126+
final IRubyObject result = instance_variable_set(name, value);
127+
// ensure we make latest value visible
128+
UnsafeHolder.storeFence();
129+
return result;
130+
} else {
131+
throw new UnsupportedOperationException();
132+
}
133+
}
134+
}
135+
136+
@JRubyClass(name = "Object", parent = "JRubyObject")
137+
public static class Object extends JRubyObject {
138+
139+
public Object(Ruby runtime, RubyClass metaClass) {
140+
super(runtime, metaClass);
141+
}
142+
}
143+
144+
@JRubyClass(name = "AbstractLockableObject", parent = "Object")
145+
public static class AbstractLockableObject extends Object {
146+
147+
public AbstractLockableObject(Ruby runtime, RubyClass metaClass) {
148+
super(runtime, metaClass);
149+
}
150+
}
151+
152+
@JRubyClass(name = "JRubyLockableObject", parent = "AbstractLockableObject")
153+
public static class JRubyLockableObject extends JRubyObject {
154+
155+
public JRubyLockableObject(Ruby runtime, RubyClass metaClass) {
156+
super(runtime, metaClass);
157+
}
158+
61159
@JRubyMethod(name = "synchronize", visibility = Visibility.PROTECTED)
62160
public IRubyObject rubySynchronize(ThreadContext context, Block block) {
63161
synchronized (this) {
@@ -108,58 +206,5 @@ public IRubyObject nsBroadcast(ThreadContext context) {
108206
notifyAll();
109207
return this;
110208
}
111-
112-
@JRubyMethod(name = "ensure_ivar_visibility!", visibility = Visibility.PROTECTED)
113-
public IRubyObject ensureIvarVisibilityBang(ThreadContext context) {
114-
if (UnsafeHolder.U == null) {
115-
// We are screwed
116-
throw new UnsupportedOperationException();
117-
} else if (UnsafeHolder.SUPPORTS_FENCES)
118-
// We have to prevent ivar writes to reordered with storing of the final instance reference
119-
// Therefore wee need a fullFence to prevent reordering in both directions.
120-
UnsafeHolder.fullFence();
121-
else {
122-
// Assumption that this is not eliminated, if false it will break non x86 platforms.
123-
UnsafeHolder.U.putIntVolatile(this, AN_VOLATILE_FIELD_OFFSET, 1);
124-
UnsafeHolder.U.getIntVolatile(this, AN_VOLATILE_FIELD_OFFSET);
125-
}
126-
return context.nil;
127-
}
128-
129-
@JRubyMethod(name = "instance_variable_get_volatile", visibility = Visibility.PROTECTED)
130-
public IRubyObject instanceVariableGetVolatile(ThreadContext context, IRubyObject name) {
131-
if (UnsafeHolder.U == null) {
132-
// TODO: Possibly dangerous, there may be a deadlock on the this
133-
synchronized (this) {
134-
return instance_variable_get(context, name);
135-
}
136-
} else if (UnsafeHolder.SUPPORTS_FENCES) {
137-
// ensure we see latest value
138-
UnsafeHolder.loadFence();
139-
return instance_variable_get(context, name);
140-
} else {
141-
UnsafeHolder.U.getIntVolatile(this, AN_VOLATILE_FIELD_OFFSET);
142-
return instance_variable_get(context, name);
143-
}
144-
}
145-
146-
@JRubyMethod(name = "instance_variable_set_volatile", visibility = Visibility.PROTECTED)
147-
public IRubyObject InstanceVariableSetVolatile(ThreadContext context, IRubyObject name, IRubyObject value) {
148-
if (UnsafeHolder.U == null) {
149-
// TODO: Possibly dangerous, there may be a deadlock on the this
150-
synchronized (this) {
151-
return instance_variable_set(name, value);
152-
}
153-
} else if (UnsafeHolder.SUPPORTS_FENCES) {
154-
final IRubyObject result = instance_variable_set(name, value);
155-
// ensure we make latest value visible
156-
UnsafeHolder.storeFence();
157-
return result;
158-
} else {
159-
final IRubyObject result = instance_variable_set(name, value);
160-
UnsafeHolder.U.putIntVolatile(this, AN_VOLATILE_FIELD_OFFSET, 1);
161-
return result;
162-
}
163-
}
164209
}
165210
}

lib/concurrent/agent.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
require 'concurrent/atomic/thread_local_var'
44
require 'concurrent/collection/copy_on_write_observer_set'
55
require 'concurrent/concern/observable'
6-
require 'concurrent/synchronization/object'
6+
require 'concurrent/synchronization'
77

88
module Concurrent
99

lib/concurrent/atom.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
require 'concurrent/atomic/atomic_reference'
22
require 'concurrent/collection/copy_on_notify_observer_set'
33
require 'concurrent/concern/observable'
4-
require 'concurrent/synchronization/object'
4+
require 'concurrent/synchronization'
55

66
module Concurrent
77

@@ -76,7 +76,7 @@ class Atom < Synchronization::Object
7676
#
7777
# @raise [ArgumentError] if the validator is not a `Proc` (when given)
7878
def initialize(value, opts = {})
79-
@Validator = opts.fetch(:validator, -> (v) { true })
79+
@Validator = opts.fetch(:validator, -> v { true })
8080
self.observers = Collection::CopyOnNotifyObserverSet.new
8181
super(value) # ensures visibility
8282
end

lib/concurrent/executor/abstract_executor_service.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
require 'concurrent/errors'
22
require 'concurrent/executor/executor_service'
3-
require 'concurrent/synchronization/object'
3+
require 'concurrent/synchronization'
44
require 'concurrent/utility/at_exit'
55

66
module Concurrent

lib/concurrent/executor/safe_task_executor.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
require 'concurrent/synchronization/object'
1+
require 'concurrent/synchronization'
22

33
module Concurrent
44

lib/concurrent/executor/serialized_execution.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
require 'concurrent/errors'
22
require 'concurrent/concern/logging'
3-
require 'concurrent/synchronization/object'
3+
require 'concurrent/synchronization'
44

55
module Concurrent
66

lib/concurrent/utility/native_extension_loader.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
require 'concurrent/synchronization/abstract_object' # for JRuby
1+
require 'concurrent/synchronization' # has to be loaded before JRuby extensions
22
require 'concurrent/utility/engine'
33

44
module Concurrent

0 commit comments

Comments
 (0)