Skip to content

Commit

Permalink
Remove Unsafe usage for varTable management
Browse files Browse the repository at this point in the history
This switches all RubyBasicObject.varTable management away from
Unsafe and toward the equivalent VarHandle forms.
  • Loading branch information
headius committed Dec 11, 2024
1 parent 9c70e01 commit 145273b
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 371 deletions.
10 changes: 4 additions & 6 deletions core/src/main/java/org/jruby/RubyBasicObject.java
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,6 @@ public class RubyBasicObject implements Cloneable, IRubyObject, Serializable, Co
/** locking stamp for Unsafe ops updating the vartable */
public transient volatile int varTableStamp;

/** offset of the varTable field in RubyBasicObject */
public static final long VAR_TABLE_OFFSET = UnsafeHolder.fieldOffset(RubyBasicObject.class, "varTable");

/** offset of the varTableTamp field in RubyBasicObject */
public static final long STAMP_OFFSET = UnsafeHolder.fieldOffset(RubyBasicObject.class, "varTableStamp");

/**
* The error message used when some one tries to modify an
* instance variable in a high security setting.
Expand Down Expand Up @@ -3186,4 +3180,8 @@ final RubyBasicObject infectBy(int tuFlags) {
public static final int COMPARE_BY_IDENTITY_F = USER8_F;
@Deprecated
public static final int TAINTED_F = 0;
@Deprecated
public static final long VAR_TABLE_OFFSET = -1;
@Deprecated
public static final long STAMP_OFFSET = -1;
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
import org.jruby.RubyBasicObject;
import org.jruby.RubyClass;
import org.jruby.util.ArraySupport;
import org.jruby.util.unsafe.UnsafeHolder;

import java.lang.invoke.VarHandle;

/**
* A variable accessor that uses a stamped volatile int and Unsafe methods to
Expand Down Expand Up @@ -92,22 +93,21 @@ public static void setVariable(RubyBasicObject self, RubyClass realClass, int in
// spin-wait if odd
if((currentStamp & 0x01) != 0)
continue;
Object[] currentTable = (Object[]) UnsafeHolder.U.getObjectVolatile(self, RubyBasicObject.VAR_TABLE_OFFSET);

Object[] currentTable = (Object[]) VAR_TABLE_HANDLE.getVolatile(self);

if (currentTable == null || index >= currentTable.length) {
if (!createTableUnsafe(self, currentStamp, realClass, currentTable, index, value)) continue;
if (!createTable(self, currentStamp, realClass, currentTable, index, value)) continue;
} else {
if (!updateTableUnsafe(self, currentStamp, currentTable, index, value)) continue;
if (!updateTable(self, currentStamp, currentTable, index, value)) continue;
}

break;
}
}

/**
* Create or exapand a table for the given object, using Unsafe CAS and
* ordering operations to ensure visibility.
* Create or expand a table for the given object, using VarHandle ordering operations to ensure visibility.
*
* @param self the object into which to set the variable
* @param currentStamp the current variable table stamp
Expand All @@ -117,9 +117,9 @@ public static void setVariable(RubyBasicObject self, RubyClass realClass, int in
* @param value the variable's value
* @return whether the update was successful, for CAS retrying
*/
private static boolean createTableUnsafe(RubyBasicObject self, int currentStamp, RubyClass realClass, Object[] currentTable, int index, Object value) {
private static boolean createTable(RubyBasicObject self, int currentStamp, RubyClass realClass, Object[] currentTable, int index, Object value) {
// try to acquire exclusive access to the varTable field
if (!UnsafeHolder.U.compareAndSwapInt(self, RubyBasicObject.STAMP_OFFSET, currentStamp, ++currentStamp)) {
if (!STAMP_HANDLE.compareAndSet(self, currentStamp, ++currentStamp)) {
return false;
}

Expand All @@ -130,8 +130,8 @@ private static boolean createTableUnsafe(RubyBasicObject self, int currentStamp,
}

newTable[index] = value;
UnsafeHolder.U.putOrderedObject(self, RubyBasicObject.VAR_TABLE_OFFSET, newTable);

VAR_TABLE_HANDLE.setRelease(self, newTable);

// release exclusive access
self.varTableStamp = currentStamp + 1;
Expand All @@ -140,8 +140,7 @@ private static boolean createTableUnsafe(RubyBasicObject self, int currentStamp,
}

/**
* Update the given table table for the given object, using Unsafe fence or
* volatile operations to ensure visibility.
* Update the given table for the given object, using VarHandle.fullFence to ensure visibility.
*
* @param self the object into which to set the variable
* @param currentStamp the current variable table stamp
Expand All @@ -150,10 +149,10 @@ private static boolean createTableUnsafe(RubyBasicObject self, int currentStamp,
* @param value the variable's value
* @return whether the update was successful, for CAS retrying
*/
private static boolean updateTableUnsafe(RubyBasicObject self, int currentStamp, Object[] currentTable, int index, Object value) {
private static boolean updateTable(RubyBasicObject self, int currentStamp, Object[] currentTable, int index, Object value) {
// shared access to varTable field.
currentTable[index] = value;
UnsafeHolder.U.fullFence();
VarHandle.fullFence();

// validate stamp. redo on concurrent modification
return self.varTableStamp == currentStamp;
Expand Down
Loading

0 comments on commit 145273b

Please sign in to comment.