Synchronize state in AbstractVariable and Argv

These fields are changed together but without any synchronization or atomicity. Simplest fix for now is to synchronize the methods and mark them as volatile. Fixes jruby#8178
headius · May 9, 2024 · 9af8a68 · 9af8a68
1 parent d7358ea
commit 9af8a68
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 11 deletions.
diff --git a/core/src/main/java/org/jruby/embed/variable/AbstractVariable.java b/core/src/main/java/org/jruby/embed/variable/AbstractVariable.java
@@ -49,10 +49,10 @@ abstract class AbstractVariable implements BiVariable {
      */
     protected final IRubyObject receiver;
     protected final String name;
-    protected Object javaObject = null;
-    protected Class javaType = null;
-    protected IRubyObject rubyObject = null;
-    protected boolean fromRuby;
+    protected volatile Object javaObject;
+    protected volatile Class javaType;
+    protected volatile IRubyObject rubyObject;
+    protected volatile boolean fromRuby;
 
     /**
      * Constructor used when this variable is originaed from Java.
@@ -94,7 +94,7 @@ static RubyObject getTopSelf(final IRubyObject receiver) {
         return (RubyObject) receiver.getRuntime().getTopSelf();
     }
 
-    protected void updateByJavaObject(final Ruby runtime, Object... values) {
+    protected synchronized void updateByJavaObject(final Ruby runtime, Object... values) {
         assert values != null;
         javaObject = values[0];
         if (javaObject == null) {
@@ -108,7 +108,7 @@ protected void updateByJavaObject(final Ruby runtime, Object... values) {
         fromRuby = false;
     }
 
-    protected void updateRubyObject(final IRubyObject rubyObject) {
+    protected synchronized void updateRubyObject(final IRubyObject rubyObject) {
         if ( rubyObject == null ) return;
         this.rubyObject = rubyObject;
         this.javaType = null;
@@ -134,7 +134,7 @@ public String getName() {
         return name;
     }
 
-    public Object getJavaObject() {
+    public synchronized Object getJavaObject() {
         if (rubyObject == null) return javaObject;
 
         if (javaType != null) { // Java originated variables

diff --git a/core/src/main/java/org/jruby/embed/variable/Argv.java b/core/src/main/java/org/jruby/embed/variable/Argv.java
@@ -114,7 +114,7 @@ public static boolean isValidName(Object name) {
      * invoked during EvalUnit#run() is executed.
      */
     @Override
-    public void inject() {
+    public synchronized void inject() {
         final Ruby runtime = getRuntime();
 
         final RubyArray argv = RubyArray.newArray(runtime);
@@ -141,7 +141,7 @@ else if ( javaObject instanceof String[] ) {
      * this variable in top self.
      */
     @Override
-    public void remove() {
+    public synchronized void remove() {
         this.javaObject = new ArrayList();
         inject();
     }
@@ -173,7 +173,7 @@ private static void updateARGV(final IRubyObject receiver, final BiVariableMap v
     }
 
     // ARGV appears to require special treatment, leaving javaType intact
-    protected void updateRubyObject(final IRubyObject rubyObject) {
+    protected synchronized void updateRubyObject(final IRubyObject rubyObject) {
         if ( rubyObject == null ) return;
         this.rubyObject = rubyObject;
     }
@@ -193,7 +193,7 @@ public static void retrieveByKey(RubyObject receiver, BiVariableMap vars, String
 
     @Override
     @SuppressWarnings("unchecked")
-    public Object getJavaObject() {
+    public synchronized Object getJavaObject() {
         if ( rubyObject == null || ! fromRuby ) return javaObject;
 
         final RubyArray ary = (RubyArray) rubyObject;