Skip to content

Commit 2cad2f4

Browse files
authored
Merge pull request #2791 from DataDog/valpertui/fix/scroll-view-swizzle-reentrancy
2 parents 9edee5f + 5123a8c commit 2cad2f4

3 files changed

Lines changed: 145 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# Unreleased
22

3+
- [FIX] Fix stack overflow crash when RUM scroll tracking is used alongside third-party delegate proxy libraries (e.g. RxSwift). See [#2791][]
4+
35
# 3.8.3 / 26-03-2026
46

57
- [FIX] Fix TOC/TOU race in RUM scroll tracking. See [#2776][]
@@ -1094,6 +1096,7 @@ Release `2.0` introduces breaking changes. Follow the [Migration Guide](MIGRATIO
10941096
[#2750]: https://github.com/DataDog/dd-sdk-ios/pull/2750
10951097
[#2751]: https://github.com/DataDog/dd-sdk-ios/pull/2751
10961098
[#2776]: https://github.com/DataDog/dd-sdk-ios/pull/2776
1099+
[#2791]: https://github.com/DataDog/dd-sdk-ios/pull/2791
10971100

10981101
[@00fa9a]: https://github.com/00FA9A
10991102
[@britton-earnin]: https://github.com/Britton-Earnin

DatadogRUM/Sources/Instrumentation/Actions/UIKit/UIScrollViewSwizzler.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,13 @@ internal final class UIScrollViewSwizzler {
5151
/// original delegate is gone.
5252
private static var proxyKey: Void?
5353

54+
/// Set of scroll views that are currently having their delegate set.
55+
/// Detects and breaks re-entrant setter calls from third-party delegate proxies
56+
/// (e.g. RxSwift's `DelegateProxy`) that receive our proxy and re-call this setter
57+
/// via ObjC dispatch. UIKit setter calls happen on the main thread, so a plain
58+
/// `Set` is safe here.
59+
private static var scrollViewsBeingSet: Set<ObjectIdentifier> = []
60+
5461
init(handler: UIScrollViewHandler) throws {
5562
self.method = try dd_class_getInstanceMethod(UIScrollView.self, Self.selector)
5663
self.handler = handler
@@ -78,6 +85,18 @@ internal final class UIScrollViewSwizzler {
7885
return
7986
}
8087

88+
// Re-entrancy guard: prevents infinite recursion when a third-party swizzle
89+
// (e.g. RxSwift's DelegateProxy) receives our proxy and re-calls this setter
90+
// via ObjC dispatch with a different delegate type. Keyed on object identity
91+
// so independently managed scroll views do not interfere with each other.
92+
let scrollViewID = ObjectIdentifier(scrollView)
93+
guard !Self.scrollViewsBeingSet.contains(scrollViewID) else {
94+
previousImplementation(scrollView, Self.selector, delegate)
95+
return
96+
}
97+
Self.scrollViewsBeingSet.insert(scrollViewID)
98+
defer { Self.scrollViewsBeingSet.remove(scrollViewID) }
99+
81100
// Check if this delegate already has a proxy attached to it
82101
if let existingProxy = objc_getAssociatedObject(delegate, &Self.proxyKey) as? UIScrollViewDelegateProxy {
83102
// Reuse the existing proxy but update the handler in case RUM was

DatadogRUM/Tests/Instrumentation/Actions/UIKit/UIScrollViewSwizzlerTests.swift

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,129 @@ class UIScrollViewSwizzlerTests: XCTestCase {
113113
XCTAssertFalse(scrollView.delegate?.responds(to: selector) ?? false)
114114
}
115115

116+
// MARK: - Setter re-entrancy guard (regression for RxSwift-style setter re-entry crash)
117+
118+
func testSetterReentrancyGuard_whenThirdPartySwizzleReCallsSetterViaDispatch_doesNotCauseStackOverflow() throws {
119+
// Regression test for infinite recursion when a third-party library swizzles
120+
// UIScrollView.delegate setter (installed before Datadog) and, from within
121+
// Datadog's `previousImplementation` call, re-calls `scrollView.delegate = itsProxy`
122+
// via full ObjC dispatch — re-entering Datadog's swizzle before it has returned.
123+
//
124+
// Swizzle chain (Datadog last = first to fire):
125+
// [Datadog.SetDelegate] → [ThirdParty swizzle] → [original UIScrollView setter]
126+
//
127+
// Without the scrollViewsBeingSet guard (would crash with stack overflow):
128+
// 1. App sets delegate → Datadog fires → creates DDProxy → calls previousImpl (ThirdParty)
129+
// 2. ThirdParty wraps DDProxy in txProxy → dispatch: scrollView.delegate = txProxy
130+
// 3. Datadog fires again → creates DDProxy2 → calls previousImpl (ThirdParty) → … ∞
131+
//
132+
// With the guard (correct):
133+
// 1. App sets delegate → Datadog fires → inserts scrollView in set → creates DDProxy → calls previousImpl
134+
// 2. ThirdParty wraps DDProxy → dispatch: scrollView.delegate = txProxy
135+
// 3. Datadog fires → scrollView IS in set → guard fires → passes txProxy through directly
136+
// 4. Chain resolves; defer removes scrollView from set
137+
138+
guard let handler else {
139+
XCTFail("Handler should be initialized")
140+
return
141+
}
142+
143+
// Capture the current setter IMP before any test swizzle is installed
144+
let setterSel = #selector(setter: UIScrollView.delegate)
145+
guard let setterMethod = class_getInstanceMethod(UIScrollView.self, setterSel) else {
146+
XCTFail("UIScrollView.delegate setter method not found in ObjC runtime")
147+
return
148+
}
149+
let savedIMP = method_getImplementation(setterMethod)
150+
151+
// Build and install a raw ObjC-style "third-party" setter swizzle.
152+
// This simulates the pattern used by RxSwift's DelegateProxy:
153+
// - If the incoming delegate is NOT already a ThirdPartyDelegateProxy:
154+
// wrap it in one and re-call the setter via ObjC dispatch (the dangerous re-entry).
155+
// - If it already IS a ThirdPartyDelegateProxy: pass through to the previous IMP.
156+
typealias SetterCIMP = @convention(c) (UIScrollView, Selector, UIScrollViewDelegate?) -> Void
157+
158+
// Holds the IMP that was current before the third-party swizzle was installed,
159+
// so the pass-through branch can call it correctly.
160+
class IMPHolder { var imp: IMP? }
161+
let prevHolder = IMPHolder()
162+
163+
// Holds a strong reference to the ThirdPartyDelegateProxy created in the block.
164+
// UIScrollView.delegate is a weak property, so without this the proxy would be
165+
// deallocated immediately after the block exits — just as in real frameworks the
166+
// DelegateProxy is retained by the observable chain.
167+
class ProxyHolder { var proxy: ThirdPartyDelegateProxy? }
168+
let proxyHolder = ProxyHolder()
169+
170+
let thirdPartyBlock: @convention(block) (UIScrollView, UIScrollViewDelegate?) -> Void = { scrollView, delegate in
171+
guard let delegate = delegate, !(delegate is ThirdPartyDelegateProxy) else {
172+
// Pass through to whatever IMP was current before the third-party installed
173+
if let prev = prevHolder.imp {
174+
unsafeBitCast(prev, to: SetterCIMP.self)(scrollView, setterSel, delegate)
175+
}
176+
return
177+
}
178+
// Simulate RxSwift DelegateProxy: wrap the delegate and re-call via ObjC dispatch.
179+
// This call goes through the full swizzle chain from the top (re-enters Datadog's swizzle).
180+
let thirdPartyProxy = ThirdPartyDelegateProxy()
181+
thirdPartyProxy.forwardToDelegate = delegate as? (NSObject & UIScrollViewDelegate)
182+
proxyHolder.proxy = thirdPartyProxy // Retain so the weak scrollView.delegate stays alive
183+
scrollView.delegate = thirdPartyProxy
184+
}
185+
let thirdPartyIMP = imp_implementationWithBlock(thirdPartyBlock)
186+
// Install the third-party swizzle; record the IMP it replaces for pass-through
187+
prevHolder.imp = method_setImplementation(setterMethod, thirdPartyIMP)
188+
189+
// Now install Datadog's swizzle (it captures thirdPartyIMP as its previousImplementation)
190+
swizzler = try UIScrollViewSwizzler(handler: handler)
191+
swizzler?.swizzle()
192+
193+
defer {
194+
// Cleanup in reverse installation order:
195+
// 1. Remove Datadog's swizzle (restores method IMP to thirdPartyIMP)
196+
swizzler?.unswizzle()
197+
swizzler = nil // Prevent double-unswizzle in tearDown
198+
// 2. Restore the IMP that was in place before the third-party test swizzle
199+
method_setImplementation(setterMethod, savedIMP)
200+
}
201+
202+
let scrollView = UIScrollView()
203+
let originalDelegate = MockScrollViewDelegate()
204+
205+
// When — must not cause infinite recursion / stack overflow
206+
scrollView.delegate = originalDelegate
207+
208+
// Then — the delegate chain was established without crashing
209+
XCTAssertNotNil(scrollView.delegate)
210+
}
211+
212+
func testSetterReentrancyGuard_isReleasedAfterEachCall_soSubsequentIndependentCallsWork() throws {
213+
// Verify that the scrollViewsBeingSet guard is cleared (via defer) after each
214+
// completed setter call, so that a second, independent call to the same scroll
215+
// view's delegate setter is not mistakenly treated as re-entrant.
216+
//
217+
// If the guard were not cleared (e.g. defer missing), the second call would hit
218+
// the guard and bypass proxy creation, breaking scroll-event tracking for that view.
219+
220+
guard let handler else {
221+
XCTFail("Handler should be initialized")
222+
return
223+
}
224+
swizzler = try UIScrollViewSwizzler(handler: handler)
225+
swizzler?.swizzle()
226+
227+
let scrollView = UIScrollView()
228+
let delegate1 = MockScrollViewDelegate()
229+
let delegate2 = MockScrollViewDelegate()
230+
231+
// When — two independent (non-re-entrant) setter calls in sequence
232+
scrollView.delegate = delegate1
233+
scrollView.delegate = delegate2
234+
235+
// Then — the second assignment takes effect; getter returns delegate2 (not delegate1)
236+
XCTAssertTrue(scrollView.delegate === delegate2)
237+
}
238+
116239
// MARK: - Double-Wrap Prevention
117240

118241
func testDoubleWrapPrevention_whenDelegateIsAlreadyProxy_itDoesNotWrapAgain() throws {

0 commit comments

Comments
 (0)