Replace JavaFinalize() with Dispose(bool) in GenericAnimatorListener

[jonathanpeppers]

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Description

Replace JavaFinalize() override with Dispose(bool disposing) in GenericAnimatorListener.

JavaFinalize() maps to Java's Object.finalize() which has been deprecated since JDK 9 and is marked for removal. Additionally, JavaFinalize() can fire while C# still holds live references to the object, leading to premature cleanup of delegate references.

The recommended pattern is to use Dispose(bool disposing) instead, which integrates properly with .NET's disposable lifecycle.

See dotnet/android#2072 for context on the JavaFinalize() deprecation.

Changes

• Replaced JavaFinalize() override with Dispose(bool disposing) in GenericAnimatorListener.cs
• Delegate references (OnCancel, OnRepeat, OnEnd) are now cleared only during managed disposal (disposing == true)

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35548

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35548"

[kubaflo]

/review -b feature/regression-check -p android

[MauiBot]

🤖 AI Summary

👋 @jonathanpeppers — new AI review results are available. Please review the latest session below.

📊 Review Session — 924ad26 · Replace JavaFinalize() with Dispose(bool) in GenericAnimatorListener · 2026-05-20 20:00 UTC

🚦 Gate — Test Before & After Fix

Gate Result: ⚠️ SKIPPED

No tests were detected in this PR.

Recommendation: Add tests to verify the fix using the write-tests-agent.

---

🧪 UI Tests

Full UI test matrix will run (no specific categories detected from PR changes).

---

🔍 Pre-Flight — Context & Validation

Pre-Flight — PR #35548

PR Summary

Title: Replace JavaFinalize() with Dispose(bool) in GenericAnimatorListener
Author: @jonathanpeppers
Branch: jonathanpeppers/literate-disco → main
File touched: src/Controls/src/Core/Platform/Android/GenericAnimatorListener.cs (+6/-3)

Motivation

• JavaFinalize() corresponds to Java's deprecated Object.finalize() (deprecated since JDK 9, scheduled for removal — see Members of a singleTop activity in background are finalized android#2072).
• JavaFinalize() can run while C# still holds live references, allowing premature delegate-field clearing.
• Recommended pattern in Java.Lang.Object-derived classes is Dispose(bool disposing).

Diff (PR's current fix)

-		protected override void JavaFinalize()
+		protected override void Dispose(bool disposing)
 		{
-			OnCancel = OnRepeat = OnEnd = null;
-			base.JavaFinalize();
+			if (disposing)
+			{
+				OnCancel = OnRepeat = OnEnd = null;
+			}
+			base.Dispose(disposing);
 		}

Usages of GenericAnimatorListener

• src/Compatibility/Core/src/Android/AppCompat/Platform.cs:149 — modal dismiss animation; SetListener(new GenericAnimatorListener { ... }).
• src/Compatibility/Core/src/Android/AppCompat/Platform.cs:612 — modal present animation; same pattern.
• No Dispose() is ever called on these listeners. They are constructed inline and only referenced by the Android Animator until the animation completes.

Public-API Analyzer Concern (CRITICAL)

src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Shipped.txt:8036 lists:

override Microsoft.Maui.Controls.Platform.GenericAnimatorListener.JavaFinalize() -> void

Removing this override deletes a shipped public API entry. The PR does not add *REMOVED*~override … JavaFinalize() -> void to PublicAPI.Unshipped.txt, and does not add the new Dispose(bool) override entry. This will fail the RS0016/RS0017 PublicAPI analyzers in CI.

Behavioral Concern

With Dispose(bool disposing), the delegate-clearing happens only on explicit/managed disposal. Since neither call site ever explicitly disposes the listener (see Platform.cs above), and the listener is finalized by the Android binding runtime (which calls Dispose(false)), the delegates are never nulled at runtime under current usage. That makes the new if (disposing) branch effectively dead code for the in-repo callers, while the old JavaFinalize version did clear them (during finalization, when the cycle to C# delegate targets was about to be collected anyway).

In practice neither path is meaningful — once the listener becomes unreachable, the delegate fields' targets are also collected by .NET GC. So the null-out is more cosmetic than functional in either approach.

Tests

• Gate phase: SKIPPED — no new tests in PR.
• No existing tests reference GenericAnimatorListener.

Risks / Things to Verify

1. PublicAPI files must be updated (*REMOVED* line + new Dispose(bool) override line). Without this, CI fails.
2. The PR adds a Dispose(bool) override; if Java.Lang.Object's Dispose(bool) is protected, the analyzer still tracks it.
3. Any subclasses outside MAUI overriding JavaFinalize() on this type (theoretically possible since the type is public) would break — but very unlikely.
4. The body inside if (disposing) is essentially a no-op for current callers. Consider whether the override is needed at all.

Alternative-Approach Surface (for try-fix)

• A. Simply remove the override entirely — the GC + Android binding cleanup handles everything; the null-out is unnecessary.
• B. Keep Dispose(bool) but null in both branches (matches the original JavaFinalize semantic more closely while still adopting the recommended hook).
• C. PR's fix + the missing PublicAPI updates (purely make the PR mergeable as-is).

---

🔧 Fix — Analysis & Comparison

Try-Fix Aggregate — PR #35548

Three meaningfully different fix candidates were generated with guidance from the
maui-expert-reviewer agent (Android lifecycle / handler dimension). All three
candidates achieve the PR's stated goal of removing the JDK-9-deprecated
JavaFinalize() override (dotnet/android#2072). They differ in how — or whether —
they preserve the historical delegate-nulling behavior.

The gate phase was SKIPPED (no tests in the PR and none exist for
GenericAnimatorListener in the repo). Therefore "testing" each candidate consisted
of:

1. Verifying the in-tree call sites in
   src/Compatibility/Core/src/Android/AppCompat/Platform.cs (lines 149, 612) still
   compile and behave identically.
2. Verifying the RS0016/RS0017 PublicAPI analyzers would pass with the proposed
   PublicAPI.Unshipped.txt deltas.
3. Reasoning about the dotnet/android GC-bridge contract for Java.Lang.Object
   subclasses (Dispose(false) fires only when both peers are unreachable, so the
   nulling is functionally redundant in every candidate).

Key finding shared by all candidates

The PR as submitted is missing required PublicAPI.Unshipped.txt entries and
will fail CI's RS0016/RS0017 analyzers:

*REMOVED*override Microsoft.Maui.Controls.Platform.GenericAnimatorListener.JavaFinalize() -> void
override Microsoft.Maui.Controls.Platform.GenericAnimatorListener.Dispose(bool disposing) -> void

(The second line is only needed for Try-Fix 1; Try-Fix 2 and Try-Fix 3 don't add a
Dispose(bool) override and therefore only need the *REMOVED* line.)

Candidate summary

#	Approach	Adds new public API?	Behavior change vs. shipped code	Recommended
1	Keep PR's Dispose(bool) + add missing PublicAPI lines	Yes (Dispose(bool))	None observable in repo	✅
2	Delete the override entirely; rely on GC	No	None observable in repo	⚠️ defensible
3	Self-cleaning callbacks (null delegates after invoke); delete override	No	Fire-once contract on OnEnd/OnCancel	❌ out of scope

Recommendation

Try-Fix 1 is the minimum-correct, mergeable form of the PR. It preserves the
author's intent, fixes the analyzer-blocking omission, and follows the
codebase-wide convention used by every other Java.Lang.Object-derived class with
managed-reference cleanup (e.g. FrameRenderer, ItemsViewAdapter,
RecyclerViewScrollListener).

Try-Fix 2 is a reasonable simplification if the project prefers to minimize new
public-API surface. The trade-off is purely stylistic (convention vs. surface).

Try-Fix 3 correctly addresses the root reference-cycle concern but changes
observable callback semantics on a public type. It should land as a separate,
documented change rather than being folded into a deprecation-cleanup PR.

Files

• try-fix-1/content.md — Alternative B (recommended)
• try-fix-2/content.md — Alternative A (delete override)
• try-fix-3/content.md — Alternative D (self-cleaning callbacks)
• pre-flight/content.md — Context gathering

---

📋 Report — Final Recommendation

Comparative Report — PR #35548

PR: Replace JavaFinalize() with Dispose(bool) in GenericAnimatorListener
Author: @jonathanpeppers
Platform under test: android
Gate: SKIPPED (no tests in PR; no existing tests for this type)

Candidates evaluated

ID	Source-code change	PublicAPI deltas	CI status (analyzers)	Regression risk
pr	PR as submitted (JavaFinalize→Dispose(bool) w/ if (disposing))	none	❌ FAILS RS0016+RS0017	low
pr-plus-reviewer	identical to pr	adds *REMOVED*…JavaFinalize + …Dispose(bool disposing)	✅ passes	low
try-fix-1	identical to pr	adds *REMOVED*…JavaFinalize + …Dispose(bool disposing)	✅ passes	low
try-fix-2	deletes override entirely	adds only *REMOVED*…JavaFinalize	✅ passes	low
try-fix-3	self-cleaning callbacks (null delegates on terminal callback) + delete override	adds only *REMOVED*…JavaFinalize	✅ passes	medium (changes observable contract of public type)

Gate was SKIPPED for all candidates (no regression tests exist for
GenericAnimatorListener), so no candidate "failed" regression tests in the
strict sense. The ranking below uses analyzer-pass as the equivalent gate
since RS0016/RS0017 would block merge.

Detailed comparison

pr — current PR as submitted

• ✅ Adopts correct dotnet/android idiom (Dispose(bool) over deprecated
  JavaFinalize).
• ❌ Blocking: missing PublicAPI.Unshipped.txt entries. Will fail
  RS0016 (new override not declared) and RS0017 (shipped override
  removed without *REMOVED* marker).
• ⚠️ if (disposing) body is unreachable for in-repo callers, but
  consistent with peer classes (cosmetic).
• Ranking penalty: treated as below all analyzer-passing candidates
  because it cannot land.

pr-plus-reviewer — PR + reviewer feedback

• ✅ Source change preserved.
• ✅ Both required PublicAPI lines added.
• ✅ Identical runtime behavior to PR.
• ✅ Consistent with codebase convention (matches FrameRenderer,
  ItemsViewAdapter<,>, MauiCarouselRecyclerView, etc.).
• ✅ Minimum diff to land PR.
• Content-identical to try-fix-1 — strong convergence signal that
  this is the correct fix shape.

try-fix-1 — independent generation, same outcome

• Same code as pr-plus-reviewer. Listed separately for provenance.
• Independent agent generation reached the same conclusion → high
  confidence the approach is correct.

try-fix-2 — delete override entirely

• ✅ Smaller public-API surface (no new tracked member).
• ✅ Same runtime behavior — the null-out was a no-op anyway.
• ⚠️ Diverges from codebase-wide convention of providing a Dispose(bool)
  hook on Java.Lang.Object-derived public types.
• ⚠️ Discards the author's intent (the PR title and description explicitly
  call out replacing JavaFinalize with Dispose(bool), not removing
  cleanup).
• Defensible but not preferred — would require a separate justification
  to override the author's stated intent.

try-fix-3 — self-cleaning callbacks

• ✅ Most principled fix for the underlying reference-cycle concern.
• ❌ Changes observable contract of a public type: imposes a fire-once
  semantic on OnEnd/OnCancel and wipes OnRepeat on first terminal
  callback.
• ❌ Scope creep — PR is about replacing a deprecated JVM hook, not about
  restructuring callback semantics.
• ❌ Higher regression risk: any external consumer of the public
  GenericAnimatorListener that re-uses a listener across animations would
  break.
• Out of scope for this PR. Worth filing as a follow-up if needed.

Ranking

1. 🏆 pr-plus-reviewer — PR's intent preserved, analyzer-blocking gap
   closed, idiomatic with the rest of the codebase, lowest regression risk,
   independently corroborated by try-fix-1.
2. try-fix-1 — content-identical to winner; effectively a tie. Ranked
   below only by provenance (preserves the PR author's exact source change
   vs. generated independently).
3. try-fix-2 — defensible simplification, but discards author intent
   and breaks codebase convention.
4. try-fix-3 — out of scope; changes public contract; should ship as
   a separate, documented change.
5. pr — cannot land as-is (analyzer failures). Strictly dominated by
   pr-plus-reviewer, which is the same diff plus the missing PublicAPI
   lines.

Winner: pr-plus-reviewer

The PR's source change is correct as designed; only the
PublicAPI.Unshipped.txt updates are missing. Applying the reviewer's
blocker-severity feedback makes the PR mergeable without changing intent
or runtime behavior. Independent generation (try-fix-1) converged on
exactly the same result, increasing confidence.

Recommended PR comment to author

The lifecycle change LGTM and matches the dotnet/android Dispose(bool)
convention used by other Java.Lang.Object-derived classes in this
repo. Two PublicAPI.Unshipped.txt entries are required for CI to pass
(RS0016 / RS0017):

*REMOVED*override Microsoft.Maui.Controls.Platform.GenericAnimatorListener.JavaFinalize() -> void
override Microsoft.Maui.Controls.Platform.GenericAnimatorListener.Dispose(bool disposing) -> void

Optional nit: file is missing a trailing newline.

---