Fabric: schedulerDidFinishTransaction picks oldest (not newest) pending transaction as merge target, corrupting mount order
Nadie ha tomado este issue todavía.
- Lenguaje dominante
- C++
- Estrellas
- 127k
- Forks
- 25.3k
- Merge medio
- 1 d 23 h
- PR fusionados (30 d)
- 4
Descripción
Description
FabricUIManagerBinding::schedulerDidFinishTransaction (Android, ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.cpp) selects which pending mounting transaction an incoming transaction should merge into using a forward search:
auto pendingTransaction = std::find_if(
pendingTransactions_.begin(),
pendingTransactions_.end(),
[&](const auto& transaction) {
return transaction.getSurfaceId() == mountingTransaction->getSurfaceId();
});
if (pendingTransaction != pendingTransactions_.end() &&
pendingTransaction->canMergeWith(*mountingTransaction)) {
pendingTransaction->mergeWith(std::move(*mountingTransaction));
} else {
pendingTransactions_.push_back(std::move(*mountingTransaction));
}
std::find_if over begin()/end() returns the oldest queued transaction for a surface. Whenever a surface has more than one pending transaction at once — which can legitimately happen any time MountingTransaction::canMergeWith refuses a merge between two adjacent transactions — this picks the wrong merge target.
Repro scenario
Given pending transactions T1, T2 for the same surface (already queued separately because canMergeWith refused to combine them), and an incoming T3:
T3was diffed by the renderer against shadow-tree state that already includesT2.- The forward search finds
T1first and mergesT3into it, producing[T1+T3, T2], which executes asT1 → T3 → T2. - But
T3was never diffed against a tree withoutT2in it — running it beforeT2desyncs the native view tree from the shadow tree that produced the diff.
This manifests as native-tree/shadow-tree divergence at mount-apply time: an insert lands at an index the real parent doesn't have (IndexOutOfBoundsException in addViewAt), or a remove resolves a stale parent tag that's no longer the expected ViewGroup (IllegalStateException in removeViewAt).
Expected behavior
A new transaction should only ever merge into the most recently queued pending transaction for its surface — the only one whose diffed-against tree state actually reflects everything already queued. That means the merge-target lookup should search from the back (rbegin()/rend()) instead of the front (begin()/end()).
Fix
Three-token change:
auto pendingTransaction = std::find_if(
pendingTransactions_.rbegin(),
pendingTransactions_.rend(),
[&](const auto& transaction) {
return transaction.getSurfaceId() == mountingTransaction->getSurfaceId();
});
if (pendingTransaction != pendingTransactions_.rend() &&
pendingTransaction->canMergeWith(*mountingTransaction)) {
...
Environment
React Native 0.86.0, Android (Fabric), observed downstream in a fork carrying an additional local patch that makes canMergeWith refuse certain merges (a Delete↔Create tag-pairing guard), which is what surfaces multi-entry pendingTransactions_ queues in practice. The underlying merge-target selection bug is present in schedulerDidFinishTransaction upstream regardless of that local patch — any code path that causes canMergeWith to refuse a merge (including future upstream guards) would trigger the same divergence.
Reproducibility
No deterministic manual repro is provided here — the bug is timing/queue-state dependent. The mechanism is provable deterministically with a unit test against MountingTransaction directly (construct 3 transactions, assert execution order), without needing device timing.
Guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Línea de trabajo
Comienza en ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.cpp e inspecciona schedulerDidFinishTransaction junto con MountingTransaction::canMergeWith. Construye tres transacciones en una prueba unitaria determinista, verifica el orden de ejecución para varias transacciones pendientes y confirma que el objetivo de la fusión es la transacción encolada más recientemente.
Escrito por el modelo de indexación a partir del texto del issue.
Evaluación
- Stack tecnológico
- android, cpp, react-native
- Área
- mobile, mobile-dev
- Tipo de issue
- Error
- Dificultad
- 2/5
- Tiempo estimado
- 1-3 horas
- Estado de actividad
- Activo
- Claridad
- Bien especificado
- Aptitud para principiantes
- 78/100