From 623bb15b4fda4f1913e48e735179983b9b0caca8 Mon Sep 17 00:00:00 2001
From: "Wei Zhang (Devinfra)" <zwei@meta.com>
Date: Thu, 27 Feb 2025 14:10:23 -0800
Subject: [PATCH] Perform lambda patch in its own parallel walk

Summary:
## Context

The 1st parallel [walk](https://www.internalfb.com/code/fbsource/[a5da2b900a795b6004c92bc57575d3d697445fda]/fbandroid/native/redex/opt/typedef-anno-checker/TypedefAnnoPatcher.cpp?lines=434) in the patcher tries to do too many things in one go. It could patch the same method/field in different threads via different code paths. the same field/method if patched via different code path can trigger separate logic after the patching event causing things to diverge.

## Proposal

It is still not clear to me what is the exact piece of logic that introduced the nondeterminism. But it's obvious to me that breaking things up into steps should help.

In this change, I move the more localized and straightforward patching logic into its own parallel walk. As a result, we move the lambda patching logic into its own walk minimizing the adverse interactions between the two steps.

Reviewed By: wsanville

Differential Revision: D70307382

fbshipit-source-id: d249e0da03eba64fb607b3fc400c4be9ecbb7ed0
---
 .../TypedefAnnoPatcher.cpp                    | 29 ++++++++++++-------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/opt/typedef-anno-checker/TypedefAnnoPatcher.cpp b/opt/typedef-anno-checker/TypedefAnnoPatcher.cpp
index 57205242090..3a9830fab4b 100644
--- a/opt/typedef-anno-checker/TypedefAnnoPatcher.cpp
+++ b/opt/typedef-anno-checker/TypedefAnnoPatcher.cpp
@@ -436,6 +436,24 @@ void TypedefAnnoPatcher::run(const Scope& scope) {
         if (is_enum(cls) && type::is_kotlin_class(cls)) {
           fix_kt_enum_ctor_param(cls, class_stats.fix_kt_enum_ctor_param);
         }
+        for (auto m : cls->get_all_methods()) {
+          patch_parameters_and_returns(
+              m, class_stats.patch_parameters_and_returns);
+          patch_synth_methods_overriding_annotated_methods(
+              m, class_stats.patch_synth_methods_overriding_annotated_methods);
+          if (is_constructor(m) &&
+              has_typedef_annos(m->get_param_anno(), m_typedef_annos)) {
+            patch_synth_cls_fields_from_ctor_param(
+                m, class_stats.patch_synth_cls_fields_from_ctor_param);
+          }
+        }
+        return class_stats;
+      });
+
+  m_patcher_stats +=
+      walk::parallel::classes<PatcherStats>(scope, [this](DexClass* cls) {
+        auto class_stats = PatcherStats();
+
         if (is_synthesized_lambda_class(cls) || is_fun_interface_class(cls)) {
           std::vector<const DexField*> patched_fields;
           for (auto m : cls->get_all_methods()) {
@@ -480,17 +498,6 @@ void TypedefAnnoPatcher::run(const Scope& scope) {
                 });
           }
         }
-        for (auto m : cls->get_all_methods()) {
-          patch_parameters_and_returns(
-              m, class_stats.patch_parameters_and_returns);
-          patch_synth_methods_overriding_annotated_methods(
-              m, class_stats.patch_synth_methods_overriding_annotated_methods);
-          if (is_constructor(m) &&
-              has_typedef_annos(m->get_param_anno(), m_typedef_annos)) {
-            patch_synth_cls_fields_from_ctor_param(
-                m, class_stats.patch_synth_cls_fields_from_ctor_param);
-          }
-        }
         return class_stats;
       });