Fabric: schedulerDidFinishTransaction picks oldest (not newest) pending transaction as merge target, corrupting mount order
Nessuno ha ancora preso questa issue.
- Lingua principale
- C++
- Stelle
- 127k
- Fork
- 25.3k
- Merge medio
- 1g 23h
- PR unite (30g)
- 4
Descrizione
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.
Guida per i contributori
Apri la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Direzione di ricerca
Inizia in ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.cpp e analizza schedulerDidFinishTransaction insieme a MountingTransaction::canMergeWith. Costruisci tre transazioni in un test unitario deterministico, verifica l’ordine di esecuzione per più transazioni in attesa e conferma che la destinazione del merge sia la transazione accodata più di recente.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- android, cpp, react-native
- Ambito
- mobile, mobile-dev
- Tipo di issue
- Bug
- Difficoltà
- 2/5
- Tempo stimato
- 1-3 ore
- Stato di attività
- Attiva
- Chiarezza
- Specificata chiaramente
- Idoneità per principianti
- 78/100