Replace usage of Unsafe in StringUnsafe with VarHandle

Also remove `doPrivileged` usage from `UnsafeProvider`, which has been deprecated for removal in JDK 17, to silence a warning.

Closes #25083.

PiperOrigin-RevId: 725673390
Change-Id: I26c8deec4b284193b511ad6d80c70fa87c7d11cc

src/main/java/com/google/devtools/build/lib/actions/ParameterFile.java

@@ -37,8 +37,6 @@
  */
 public class ParameterFile {
 
-  public static final StringUnsafe STRING_UNSAFE = StringUnsafe.getInstance();
-
   /** Different styles of parameter files. */
   public enum ParameterFileType {
     /**
@@ -100,7 +98,7 @@ public static void writeParameterFile(
   private static void writeContent(OutputStream out, Iterable<String> arguments)
       throws IOException {
     for (String line : arguments) {
-      out.write(STRING_UNSAFE.getInternalStringBytes(line));
+      out.write(StringUnsafe.getInternalStringBytes(line));
       out.write('\n');
     }
     out.flush();

src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java

@@ -134,7 +134,7 @@ protected synchronized byte[] getFastDigest(PathFragment path) {
     protected synchronized byte[] getDigest(PathFragment path) {
       return getDigestFunction()
           .getHashFunction()
-          .hashBytes(StringUnsafe.getInstance().getInternalStringBytes(path.getPathString()))
+          .hashBytes(StringUnsafe.getInternalStringBytes(path.getPathString()))
           .asBytes();
     }
   }

src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java

@@ -224,7 +224,7 @@ public String getFileContents() {
 
     @Override
     public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) {
-      return out -> out.write(StringUnsafe.getInstance().getInternalStringBytes(getFileContents()));
+      return out -> out.write(StringUnsafe.getInternalStringBytes(getFileContents()));
     }
 
     @Override
@@ -254,7 +254,7 @@ private static final class CompressedFileWriteAction extends FileWriteAction {
 
       // Grab the string's internal byte array. Calling getBytes() makes a copy, which can cause
       // memory spikes resulting in OOMs (b/290807073). Do not mutate this!
-      byte[] dataToCompress = StringUnsafe.getInstance().getByteArray(fileContents);
+      byte[] dataToCompress = StringUnsafe.getByteArray(fileContents);
 
       // Empirically, compressed sizes range from roughly 1/100 to 3/4 of the uncompressed size.
       // Presize on the small end to avoid over-allocating memory.
@@ -269,7 +269,7 @@ private static final class CompressedFileWriteAction extends FileWriteAction {
 
       this.compressedBytes = byteStream.toByteArray();
       this.uncompressedSize = dataToCompress.length;
-      this.coder = StringUnsafe.getInstance().getCoder(fileContents);
+      this.coder = StringUnsafe.getCoder(fileContents);
     }
 
     @Override
@@ -290,7 +290,7 @@ public String getFileContents() {
         throw new IllegalStateException(e);
       }
 
-      return StringUnsafe.getInstance().newInstance(uncompressedBytes, coder);
+      return StringUnsafe.newInstance(uncompressedBytes, coder);
     }
 
     @Override

src/main/java/com/google/devtools/build/lib/analysis/actions/LazyWriteNestedSetOfTupleAction.java

@@ -49,8 +49,7 @@ public LazyWriteNestedSetOfTupleAction(
 
   @Override
   public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) {
-    return out ->
-        out.write(StringUnsafe.getInstance().getInternalStringBytes(getContents(delimiter)));
+    return out -> out.write(StringUnsafe.getInternalStringBytes(getContents(delimiter)));
   }
 
   /** Computes the Action key for this action by computing the fingerprint for the file contents. */

src/main/java/com/google/devtools/build/lib/analysis/actions/LazyWritePathsFileAction.java

@@ -71,7 +71,7 @@ public LazyWritePathsFileAction(
 
   @Override
   public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) {
-    return out -> out.write(StringUnsafe.getInstance().getInternalStringBytes(getContents()));
+    return out -> out.write(StringUnsafe.getInternalStringBytes(getContents()));
   }
 
   /** Computes the Action key for this action by computing the fingerprint for the file contents. */

src/main/java/com/google/devtools/build/lib/authandtls/BasicHttpAuthenticationEncoder.java

@@ -36,7 +36,6 @@ public static String encode(String username, String password) {
     // basic authentication.
     return "Basic "
         + Base64.getEncoder()
-            .encodeToString(
-                StringUnsafe.getInstance().getInternalStringBytes(username + ":" + password));
+            .encodeToString(StringUnsafe.getInternalStringBytes(username + ":" + password));
   }
 }

src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java

@@ -1389,7 +1389,7 @@ public void createFile(
       makeDirectories(p.getPath());
       p.getPath().delete();
       try (OutputStream stream = p.getPath().getOutputStream()) {
-        stream.write(StringUnsafe.getInstance().getInternalStringBytes(content));
+        stream.write(StringUnsafe.getInternalStringBytes(content));
       }
       if (executable) {
         p.getPath().setExecutable(true);

src/main/java/com/google/devtools/build/lib/skyframe/serialization/strings/UnsafeStringCodec.java

@@ -38,8 +38,6 @@ public final class UnsafeStringCodec extends LeafObjectCodec<String> {
    */
   private static final UnsafeStringCodec INSTANCE = new UnsafeStringCodec();
 
-  private static final StringUnsafe STRING_UNSAFE = StringUnsafe.getInstance();
-
   public static UnsafeStringCodec stringCodec() {
     return INSTANCE;
   }
@@ -52,8 +50,8 @@ public Class<String> getEncodedClass() {
   @Override
   public void serialize(LeafSerializationContext context, String obj, CodedOutputStream codedOut)
       throws SerializationException, IOException {
-    byte coder = STRING_UNSAFE.getCoder(obj);
-    byte[] value = STRING_UNSAFE.getByteArray(obj);
+    byte coder = StringUnsafe.getCoder(obj);
+    byte[] value = StringUnsafe.getByteArray(obj);
     // Optimize for the case that coder == 0, in which case we can just write the length here,
     // potentially using just one byte. If coder != 0, we'll use 4 bytes, but that's vanishingly
     // rare.
@@ -79,6 +77,6 @@ public String deserialize(LeafDeserializationContext context, CodedInputStream c
       length = -length;
     }
     byte[] value = codedIn.readRawBytes(length);
-    return STRING_UNSAFE.newInstance(value, coder);
+    return StringUnsafe.newInstance(value, coder);
   }
 }

src/main/java/com/google/devtools/build/lib/unsafe/BUILD

@@ -22,8 +22,5 @@ java_library(
     add_opens = [
         "java.base/java.lang",
     ],
-    deps = [
-        ":unsafe-provider",
-        "//third_party:guava",
-    ],
+    deps = ["//third_party:guava"],
 )

src/main/java/com/google/devtools/build/lib/unsafe/StringUnsafe.java

@@ -13,14 +13,11 @@
 // limitations under the License.
 package com.google.devtools.build.lib.unsafe;
 
-import static com.google.devtools.build.lib.unsafe.UnsafeProvider.unsafe;
-
 import com.google.common.base.Ascii;
-
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
+import java.lang.invoke.VarHandle;
 import java.lang.reflect.Constructor;
-import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.util.Arrays;
 
@@ -38,9 +35,10 @@ public final class StringUnsafe {
   public static final byte LATIN1 = 0;
   public static final byte UTF16 = 1;
 
-  private static final StringUnsafe INSTANCE = new StringUnsafe();
-
+  private static final MethodHandle CONSTRUCTOR;
   private static final MethodHandle HAS_NEGATIVES;
+  private static final VarHandle VALUE_HANDLE;
+  private static final VarHandle CODE_HANDLE;
 
   static {
     try {
@@ -49,44 +47,23 @@ public final class StringUnsafe {
           stringCoding.getDeclaredMethod("hasNegatives", byte[].class, int.class, int.class);
       hasNegatives.setAccessible(true);
       HAS_NEGATIVES = MethodHandles.lookup().unreflect(hasNegatives);
-    } catch (ClassNotFoundException | NoSuchMethodException | IllegalAccessException e) {
-      throw new IllegalStateException(e);
-    }
-  }
 
-  private final Constructor<String> constructor;
-  private final long valueOffset;
-  private final long coderOffset;
+      Constructor<String> constructor =
+          String.class.getDeclaredConstructor(byte[].class, byte.class);
+      constructor.setAccessible(true);
+      CONSTRUCTOR = MethodHandles.lookup().unreflectConstructor(constructor);
 
-  public static StringUnsafe getInstance() {
-    return INSTANCE;
-  }
-
-  // TODO: b/386384684 - remove Unsafe usage
-  private StringUnsafe() {
-    Field valueField;
-    Field coderField;
-    try {
-      this.constructor = String.class.getDeclaredConstructor(byte[].class, byte.class);
-      valueField = String.class.getDeclaredField("value");
-      coderField = String.class.getDeclaredField("coder");
+      var stringLookup = MethodHandles.privateLookupIn(String.class, MethodHandles.lookup());
+      VALUE_HANDLE = stringLookup.unreflectVarHandle(String.class.getDeclaredField("value"));
+      CODE_HANDLE = stringLookup.unreflectVarHandle(String.class.getDeclaredField("coder"));
     } catch (ReflectiveOperationException e) {
-      throw new IllegalStateException(
-          "Bad fields/constructor: "
-              + Arrays.toString(String.class.getDeclaredConstructors())
-              + ", "
-              + Arrays.toString(String.class.getDeclaredFields()),
-          e);
+      throw new IllegalStateException(e);
     }
-    this.constructor.setAccessible(true);
-    valueOffset = unsafe().objectFieldOffset(valueField);
-    coderOffset = unsafe().objectFieldOffset(coderField);
   }
 
   /** Returns the coder used for this string. See {@link #LATIN1} and {@link #UTF16}. */
-  // TODO: b/386384684 - remove Unsafe usage
-  public byte getCoder(String obj) {
-    return unsafe().getByte(obj, coderOffset);
+  public static byte getCoder(String obj) {
+    return (byte) CODE_HANDLE.get(obj);
   }
 
   /**
@@ -95,9 +72,8 @@ public byte getCoder(String obj) {
    * <p>Use of this is unsafe. The representation may change from one JDK version to the next.
    * Ensure you do not mutate this byte array in any way.
    */
-  // TODO: b/386384684 - remove Unsafe usage
-  public byte[] getByteArray(String obj) {
-    return (byte[]) unsafe().getObject(obj, valueOffset);
+  public static byte[] getByteArray(String obj) {
+    return (byte[]) VALUE_HANDLE.get(obj);
   }
 
   /**
@@ -107,7 +83,7 @@ public byte[] getByteArray(String obj) {
    * <p>Use of this is unsafe. The representation may change from one JDK version to the next.
    * Ensure you do not mutate this byte array in any way.
    */
-  public byte[] getInternalStringBytes(String obj) {
+  public static byte[] getInternalStringBytes(String obj) {
     // This is both a performance optimization and a correctness check: internal strings must
     // always be coded in Latin-1, otherwise they have been constructed out of a non-ASCII string
     // that hasn't been converted to internal encoding.
@@ -122,7 +98,7 @@ public byte[] getInternalStringBytes(String obj) {
   }
 
   /** Returns whether the string is ASCII-only. */
-  public boolean isAscii(String obj) {
+  public static boolean isAscii(String obj) {
     // This implementation uses java.lang.StringCoding#hasNegatives, which is implemented as a JVM
     // intrinsic. On a machine with 512-bit SIMD registers, this is 5x as fast as a naive loop
     // over getByteArray(obj), which in turn is 5x as fast as obj.chars().anyMatch(c -> c > 0x7F) in
@@ -147,13 +123,15 @@ public boolean isAscii(String obj) {
    * <p>The new string shares the byte array instance, which must not be modified after calling this
    * method.
    */
-  public String newInstance(byte[] bytes, byte coder) {
+  public static String newInstance(byte[] bytes, byte coder) {
     try {
-      return constructor.newInstance(bytes, coder);
-    } catch (ReflectiveOperationException e) {
-      // The constructor never throws and has been made accessible, so this is not expected.
+      return (String) CONSTRUCTOR.invokeExact(bytes, coder);
+    } catch (Throwable e) {
+      // The constructor never throws, so this is not expected.
       throw new IllegalStateException(
           "Could not instantiate string: " + Arrays.toString(bytes) + ", " + coder, e);
     }
   }
+
+  private StringUnsafe() {}
 }

src/main/java/com/google/devtools/build/lib/unsafe/UnsafeProvider.java

@@ -16,16 +16,14 @@
 
 
 import java.lang.reflect.Field;
-import java.security.AccessController;
-import java.security.PrivilegedActionException;
-import java.security.PrivilegedExceptionAction;
 import sun.misc.Unsafe;
 
 /**
  * An accessor for Unsafe.
  *
  * <p>Used for serialization.
  */
+@SuppressWarnings("SunApi") // TODO: b/331765692 - clean this up
 public class UnsafeProvider {
 
   private static final Unsafe UNSAFE = getUnsafe();
@@ -46,28 +44,24 @@ public static long getFieldOffset(Class<?> type, String fieldName) throws NoSuch
    * location.
    */
   private static Unsafe getUnsafe() {
-    try {
-      // sun.misc.Unsafe is intentionally difficult to get a hold of - it gives us the power to
-      // do things like access raw memory and segfault the JVM.
-      return AccessController.doPrivileged(
-          new PrivilegedExceptionAction<Unsafe>() {
-            @Override
-            public Unsafe run() throws Exception {
-              Class<Unsafe> unsafeClass = Unsafe.class;
-              // Unsafe usually exists in the field 'theUnsafe', however check all fields
-              // in case it's somewhere else in this VM's version of Unsafe.
-              for (Field f : unsafeClass.getDeclaredFields()) {
-                f.setAccessible(true);
-                Object fieldValue = f.get(null);
-                if (unsafeClass.isInstance(fieldValue)) {
-                  return unsafeClass.cast(fieldValue);
-                }
-              }
-              throw new AssertionError("Failed to find sun.misc.Unsafe instance");
-            }
-          });
-    } catch (PrivilegedActionException pae) {
-      throw new AssertionError("Unable to get sun.misc.Unsafe", pae);
+    // sun.misc.Unsafe is intentionally difficult to get a hold of - it gives us the power to
+    // do things like access raw memory and segfault the JVM.
+    Class<Unsafe> unsafeClass = Unsafe.class;
+    // Unsafe usually exists in the field 'theUnsafe', however check all fields
+    // in case it's somewhere else in this VM's version of Unsafe.
+    for (Field f : unsafeClass.getDeclaredFields()) {
+      f.setAccessible(true);
+      Object fieldValue;
+      try {
+        fieldValue = f.get(null);
+      } catch (IllegalAccessException e) {
+        throw new IllegalStateException(
+            "Failed to get value of %s even though it has been made accessible".formatted(f), e);
+      }
+      if (unsafeClass.isInstance(fieldValue)) {
+        return unsafeClass.cast(fieldValue);
+      }
     }
+    throw new AssertionError("Failed to find sun.misc.Unsafe instance");
   }
 }

src/main/java/com/google/devtools/build/lib/util/StringEncoding.java

@@ -104,7 +104,7 @@ public final class StringEncoding {
    */
   public static String internalToPlatform(String s) {
     return needsReencodeForPlatform(s)
-        ? new String(STRING_UNSAFE.getInternalStringBytes(s), UTF_8)
+        ? new String(StringUnsafe.getInternalStringBytes(s), UTF_8)
         : s;
   }
 
@@ -115,7 +115,7 @@ public static String internalToPlatform(String s) {
    */
   public static String platformToInternal(String s) {
     return needsReencodeForPlatform(s)
-        ? STRING_UNSAFE.newInstance(s.getBytes(UTF_8), StringUnsafe.LATIN1)
+        ? StringUnsafe.newInstance(s.getBytes(UTF_8), StringUnsafe.LATIN1)
         : s;
   }
 
@@ -126,7 +126,7 @@ public static String platformToInternal(String s) {
    */
   public static String internalToUnicode(String s) {
     return needsReencodeForUnicode(s)
-        ? new String(STRING_UNSAFE.getInternalStringBytes(s), UTF_8)
+        ? new String(StringUnsafe.getInternalStringBytes(s), UTF_8)
         : s;
   }
 
@@ -137,12 +137,10 @@ public static String internalToUnicode(String s) {
    */
   public static String unicodeToInternal(String s) {
     return needsReencodeForUnicode(s)
-        ? STRING_UNSAFE.newInstance(s.getBytes(UTF_8), StringUnsafe.LATIN1)
+        ? StringUnsafe.newInstance(s.getBytes(UTF_8), StringUnsafe.LATIN1)
         : s;
   }
 
-  private static final StringUnsafe STRING_UNSAFE = StringUnsafe.getInstance();
-
   /**
    * The {@link Charset} with which the JVM encodes any strings passed to or returned from Java
    * (N)IO functions, command-line arguments or environment variables.
@@ -170,7 +168,7 @@ private static boolean needsReencodeForUnicode(String s) {
     if (BAZEL_UNICODE_STRINGS) {
       return false;
     }
-    return !STRING_UNSAFE.isAscii(s);
+    return !StringUnsafe.isAscii(s);
   }
 
   private StringEncoding() {}