commit 47f98c7b2d9e1475c81dcd38082f09b86ed98cdc
author Christoffer Adamsen <christofferqa@google.com> Wed Mar 26 08:42:57 2025 +0000
committer Christoffer Adamsen <christofferqa@google.com> Wed Mar 26 04:41:08 2025 -0700
tree 4eb6199af4707c9f70a0ca59aa97dca61fbaa2fc
parent 7a49dab0d2a899c0ede4390d20bbfda1aea0a25a

Do not mutate reparse context fields until parsing is complete

This fixes a concurrency issue where concurrent parsing of two methods on the same class could lead to a NullPointerException:

1. Thread t0 wants to parse A.foo(). It therefore starts parsing class A.

2. Thread t0 finishes parsing of method A.bar(). As a side effect of this it sets the `CfCode code` field to the parsed code object and the `ReparseContext context` field to null.

3. Thread t1 wants to parse A.bar(). It sees that the `ReparseContext context` field is set to null and concludes that the method has already been parsed. The thread therefore continues.

4. Thread t0 starts parsing method A.foo() and finds that it contains a JSR instruction. It therefore aborts parsing and sets the `CfCode code` field to null and the `ReparseContext context` field to non-null for all code objects belonging to the class.

5. Thread t0 starts parsing class A from scratch with usrJsrInliner=true.

6. Thread t1 calls asCfCode() on the code object of A.bar(), since it assumes that the method has already been parsed. This method does not take the lock, but since the `CfCode code` field has been unset by thread t0 in step 4, thread t1 now starts parsing class A.

As a result both threads t0 and t1 are now parsing class A concurrently, which is not thread safe.

Bug: b/406073767
Change-Id: Ie8fe0096329e7d39598eaae0628ff6f819401acb

src/main/java/com/android/tools/r8/graph/LazyCfCode.java

diff --git a/src/main/java/com/android/tools/r8/graph/LazyCfCode.java b/src/main/java/com/android/tools/r8/graph/LazyCfCode.java
index 3618df3..4b5f81b 100644
--- a/src/main/java/com/android/tools/r8/graph/LazyCfCode.java
+++ b/src/main/java/com/android/tools/r8/graph/LazyCfCode.java
@@ -153,6 +153,8 @@
       synchronized (context) {
         asCfCode();
       }
+    } else {
+      assert code != null;
     }
   }
 
@@ -173,8 +175,6 @@
     } catch (JsrEncountered e) {
       for (Code code : context.codeList) {
         code.asLazyCfCode().code = null;
-        code.asLazyCfCode().context = context;
-        code.asLazyCfCode().application = application;
       }
       try {
         parseCode(context, true, parsingOptions);
@@ -184,6 +184,11 @@
     } catch (Exception e) {
       throw new CompilationError("Could not parse code", e, origin);
     }
+    for (Code code : context.codeList) {
+      assert code.asLazyCfCode().code != null;
+      code.asLazyCfCode().application = null;
+      code.asLazyCfCode().context = null;
+    }
     assert verifyNoReparseContext(context.owner);
   }
 
@@ -226,10 +231,9 @@
 
   private void setCode(CfCode code) {
     assert this.code == null;
+    assert this.application != null;
     assert this.context != null;
     this.code = code;
-    this.context = null;
-    this.application = null;
   }
 
   @Override