Skip to content

re-thinking the CAAnimation spy logic, crash during caanimation object release, support-case-#692#99

Open
markdevocht wants to merge 2 commits into
masterfrom
bugfix/support-case-#692-CAAnimation-crash
Open

re-thinking the CAAnimation spy logic, crash during caanimation object release, support-case-#692#99
markdevocht wants to merge 2 commits into
masterfrom
bugfix/support-case-#692-CAAnimation-crash

Conversation

@markdevocht

@markdevocht markdevocht commented Feb 1, 2026

Copy link
Copy Markdown
Contributor

The problem was that the self object (delegate) was no longer valid when calling the origional method and thus crashed.
There was no way to check this validity.


The old approach:

  1. __detox_sync_prepareDelegateIfNeeded: (lines 78-107) dynamically injects methods into the delegate's class at runtime
  2. It copies the implementation from _DTXCAAnimationDelegateHelper and swizzles animationDidStop:finished:
  3. When the animation completes, the swizzled method is called on the delegate - but the delegate may be deallocated

Original: 1-to-Many (Class-level swizzling)
───────────────────
UIViewAnimationBlockDelegate CLASS
(modified once, affects ALL instances)

animationDidStart: ──swizzled──▶ __detox_sync_animationDidStart:
animationDidStop: ──swizzled──▶ __detox_sync_animationDidStop:

delegate A (instance) ─▶ Animation1
delegate B (instance) ─▶ Animation2
delegate C (instance) ─▶ Animation3

  • Problem: If delegate B is deallocated, Animation2's callbacks crash because the swizzled method still tries to call [self ...] on a dead object.

The new approach:

  1. __detox_sync_setDelegate: (lines 114-138) creates a new _DTXAnimationDelegateProxy instance for each animation
  2. The proxy stores a weak reference to the original delegate and becomes the animation's actual delegate
  3. When the animation completes, the proxy's animationDidStop:finished: is called - it checks if the original delegate is still alive before forwarding

New: 1-to-1 (Instance-level proxy)
───────────────────
Animation1 delegate ─▶ Proxy 1 origionalDelegate (weak) ─▶ delegate A (origional)
Animation2 delegate ─▶ Proxy 2 origionalDelegate (weak) ─▶ delegate B (DEALLOC!)
Animation3 delegate ─▶ Proxy 3 origionalDelegate (weak) ─▶ delegate C (origional)

Safety: If delegate B is deallocated:
Proxy 2's originalDelegate becomes nil (weak reference)
Proxy 2's animationDidStop: checks if (delegate) → false → no call → no crash
Animations 1 and 3 are unaffected

@markdevocht markdevocht changed the title re-thinking the CAAnimation spy, crash during caanimation object release, support-case-#692 re-thinking the CAAnimation spy logic, crash during caanimation object release, support-case-#692 Feb 1, 2026
@markdevocht markdevocht added the bug Something isn't working label Feb 1, 2026

@asafkorem asafkorem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally this should work, but wrote a few concerns I had while reading.. We can discuss this tomorrow and consider not replacing the delegate object and instead tracking via a CAAnimation lifecycle hook (or at least masking proxy identity/protocol checks) to avoid subtle behavior changes

Comment thread DetoxSync/DetoxSync/Spies/CAAnimation+DTXSpy.m Outdated
Comment thread DetoxSync/DetoxSync/Spies/CAAnimation+DTXSpy.m
@markdevocht markdevocht requested a review from asafkorem February 2, 2026 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants