Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8277444

Race condition on Instrumentation.retransformClasses() and class linking

    XMLWordPrintable

    Details

    • Subcomponent:
    • CPU:
      generic
    • OS:
      generic

      Description

      ADDITIONAL SYSTEM INFORMATION :
      Reproduced in both Windows and Linux, with JVM versions 8, 11 and 17

      A DESCRIPTION OF THE PROBLEM :
      There is a race condition when a class is simultaneously retransformed via the Instrumentation API and linked. While retransforming the class bytecode is reconstituted in src\hotspot\share\prims\jvmtiClassFileReconstituter.cpp, where in JvmtiClassFileReconstituter::copy_bytecodes there is a check if a method is already rewritten or not. If the flag is true, some references into the classpool have to be changed.
      Now, while linking a class in src\hotspot\share\oops\instanceKlass.cpp in InstanceKlass::link_class_impl there is a call to rewrite the class and then set the is_rewritten flag.
      If retransforming and linking both happen simultaneously the class is already (or partly) rewritten but the flag is still false. The reconstituter now does not translate the references back which will result in invalid bytecode, which will cause an VerifyError.
      Even worse, if class verifying is turned off, which it is by default for java.* classes, this will cause the JVM to crash/segfault.

      STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
      Load/link an (preferably big) class and callI Instrumentation.retransformClasses() on it simultaneously

      EXPECTED VERSUS ACTUAL BEHAVIOR :
      EXPECTED -
      Instrumentation.retransformClasses() works and does does not give invalid bytecode to the ClassFileTransformer.
      ACTUAL -
      Invalid bytecode given to ClassFileTransformer and followed by an VerifyError or JVM crash.

      ---------- BEGIN SOURCE ----------
      import java.lang.instrument.ClassFileTransformer;
      import java.lang.instrument.IllegalClassFormatException;
      import java.lang.instrument.Instrumentation;
      import java.lang.instrument.UnmodifiableClassException;
      import java.security.ProtectionDomain;
      import java.util.concurrent.ArrayBlockingQueue;
      import java.util.concurrent.BlockingQueue;
      import java.util.function.Consumer;

      import org.objectweb.asm.ClassWriter;
      import org.objectweb.asm.FieldVisitor;
      import org.objectweb.asm.MethodVisitor;
      import org.objectweb.asm.Opcodes;
      import org.objectweb.asm.Type;

      // this is a main class and a agent class in one. to start it make a jar from it with following manifest:
      /*
      Manifest-Version: 1.0
      Premain-Class: Main
      Can-Retransform-Classes: true
      Can-Redefine-Classes: true
       */
      // and then (i assume the jar is called crash.jar) start it with the command: java -javaagent:crash.jar=50 -cp crash.jar:asm-7.3.1.jar Main
      // the parameter for the agent is the delay of the agent

      // this reproducer is dependent on ASM 7.3.1 to create a huge class on the fly, to make the race condition more likely
      // get it via this link: https://repo1.maven.org/maven2/org/ow2/asm/asm/7.3.1/asm-7.3.1.jar
      public class Main {

      // MAIN CLASS BELOW
      // the main class just creates new classloaders in a loop, then loads the big class and passes it on to the agent via the QUEUE
      // a new classloader is needed so we create a new class instance which will be linked each time
      // i only loop 10 times because the race condition seems to be rarer after a certain warmup

      public static final String BIG_CLASS = "BigClass";

      public static volatile Consumer<Class<?>> consumer;

      public static void main(String[] args) throws Exception {
      for (int i = 0; i < 10; i++) {
      createNewClassLoaderClassAndCreateInstance();
      }
      }

      private static void createNewClassLoaderClassAndCreateInstance() throws Exception {
      ClassLoader classLoader = new AsmClassLoader();
      final Class<?> bigClass = Class.forName(BIG_CLASS, false, classLoader);
      if (consumer != null) {
      consumer.accept(bigClass);
      }
      final Object o = bigClass.getConstructor().newInstance();
      System.out.println(o.hashCode());
      Thread.sleep(150);
      }

      // this classloader is only used to load a single class, BigClass which is generated with ASM
      private static class AsmClassLoader extends ClassLoader {

      private static final byte[] BYTES = generateByteCode();

      protected AsmClassLoader() {
      super(null);
      }

      @Override
      protected Class<?> findClass(String name) throws ClassNotFoundException {
      if (name.equals(Main.BIG_CLASS)) {
      return createClass();
      }
      throw new ClassNotFoundException("can only load one class");
      }

      private Class<?> createClass() {
      return defineClass(Main.BIG_CLASS, BYTES, 0, BYTES.length);
      }

      private static byte[] generateByteCode() {
      ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS + ClassWriter.COMPUTE_FRAMES);
      addClass(cw);
      return cw.toByteArray();
      }

      private static void addClass(ClassWriter cw) {
      cw.visit(Opcodes.V1_8, Opcodes.ACC_PUBLIC, Main.BIG_CLASS, null, Type.getInternalName(Object.class), null);
      addField(cw);
      addConstructor(cw);
      addMethods(cw);
      cw.visitEnd();
      }

      private static void addField(ClassWriter cw) {
      final FieldVisitor fieldVisitor = cw.visitField(Opcodes.ACC_PRIVATE, "counter", Type.getDescriptor(int.class), null, 1);
      fieldVisitor.visitEnd();
      }

      private static void addConstructor(ClassWriter cw) {
      final MethodVisitor methodVisitor = cw.visitMethod(Opcodes.ACC_PUBLIC, "<init>", Type.getMethodDescriptor(Type.VOID_TYPE), null, null);
      methodVisitor.visitCode();
      methodVisitor.visitVarInsn(Opcodes.ALOAD, 0);
      methodVisitor.visitMethodInsn(Opcodes.INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
      methodVisitor.visitInsn(Opcodes.RETURN);
      methodVisitor.visitMaxs(1, 1);
      methodVisitor.visitEnd();
      }

      private static void addMethods(ClassWriter cw) {
      for (int i = 0; i < 100; i++) {
      final MethodVisitor methodVisitor = cw.visitMethod(Opcodes.ACC_PUBLIC, "method" + i, Type.getMethodDescriptor(Type.VOID_TYPE), null, null);
      methodVisitor.visitCode();
      for (int j = 0; j < 1000; j++) {
      methodVisitor.visitVarInsn(Opcodes.ALOAD, 0);
      methodVisitor.visitFieldInsn(Opcodes.GETFIELD, Main.BIG_CLASS, "counter", Type.getDescriptor(int.class));
      methodVisitor.visitVarInsn(Opcodes.ALOAD, 0);
      methodVisitor.visitFieldInsn(Opcodes.GETFIELD, Main.BIG_CLASS, "counter", Type.getDescriptor(int.class));
      methodVisitor.visitInsn(Opcodes.IADD);
      methodVisitor.visitVarInsn(Opcodes.ALOAD, 0);
      methodVisitor.visitInsn(Opcodes.SWAP);
      methodVisitor.visitFieldInsn(Opcodes.PUTFIELD, Main.BIG_CLASS, "counter", Type.getDescriptor(int.class));
      }
      methodVisitor.visitInsn(Opcodes.RETURN);
      methodVisitor.visitMaxs(1, 1);
      methodVisitor.visitEnd();
      }
      }
      }


      // AGENT CODE BELOW
      // the agent receives a class, waits a bit and then calls retransform. the timing depends a bit on jvm versions, ranging from ~70ms in jvm 8 to ~30ms in jvm 17

      private final static BlockingQueue<Class<?>> QUEUE = new ArrayBlockingQueue<>(1);
      public static int DELAY = 50;

      public static void premain(String agentArgs, Instrumentation inst) {
      if(agentArgs != null && !agentArgs.isEmpty()){
      DELAY = Integer.parseInt(agentArgs);
      }
      inst.addTransformer(new ClassFileTransformer() {
      @Override
      public byte[] transform(ClassLoader loader, String className, Class<?> classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) throws IllegalClassFormatException {
      return null;
      }
      });
      final Thread thread = new Thread(() -> consumeAndRetransformClasses(inst));
      thread.setDaemon(true);
      thread.start();
      Main.consumer = aClass -> {
      try {
      QUEUE.put(aClass);
      } catch (InterruptedException e) {
      e.printStackTrace();
      }
      };
      }

      private static void consumeAndRetransformClasses(Instrumentation inst) {
      while (true) {
      try {
      final Class<?> aClass = QUEUE.take();
      Thread.sleep(DELAY); //maybe fiddle with this delay. something between 30 and 70 seems to work most of the time
      inst.retransformClasses(aClass);
      } catch (UnmodifiableClassException | InterruptedException | VerifyError e) {
      e.printStackTrace();
      }
      }
      }

      }

      ---------- END SOURCE ----------

      CUSTOMER SUBMITTED WORKAROUND :
      Thread.sleep(1000) before calling retransformClasses() to wait for linking to be finished, since we cannot check if a class is already linked or not. This is not a guaranteed workaround.

      FREQUENCY : rarely


        Attachments

          Activity

            People

            Assignee:
            sspitsyn Serguei Spitsyn
            Reporter:
            webbuggrp Webbug Group
            Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

              Dates

              Created:
              Updated: