refactor: collapse blocker install check+insert into a helper - #5
Merged
Merged
Conversation
ConsentManager.update(settings:type:) is invoked on the URLSession delegate thread after remote settings are fetched. On that path it iterates destinations and calls destination.add(plugin:), which mutates analytics-swift's Mediator plugin array. Meanwhile, event processing on the analytics event thread iterates the same Mediator array via Timeline/Mediator.execute(). Because Mediator's underlying storage is a plain Swift Array, the concurrent mutation and iteration can trigger a CoW reallocation while another thread is reading a retained element, producing _swift_release_dealloc on a freed object and an EXC_BAD_ACCESS (pointer authentication failure) in release builds. Stack signature observed from a production crash: _swift_release_dealloc Mediator.add(plugin:) Timeline.swift DestinationPlugin.add(plugin:) Plugins.swift ConsentManager.update(settings:type:) Analytics.checkSettings HTTPClient.settingsFor completion com.apple.NSURLSession-delegate thread Hop the destination-mutating portion of update() onto a dedicated serial queue so those add(plugin:) calls can no longer race the event timeline. This also fixes a pre-existing check-then-insert TOCTOU that could install duplicate blockers when settings fetches overlap. The underlying thread-safety issue lives in analytics-swift Mediator and should be fixed there as well; this change is a localized workaround so consumers can upgrade immediately.
didiergarcia
changed the title
…cessing updateQueue.async returned before the destination blockers were installed, leaving a window where events could flow through destinations unguarded after checkSettings completion called resumeEventProcessing. Use .sync so update(settings:) blocks until blockers are in place, matching the original ordering contract. The serial queue still eliminates the writer/writer race on Mediator under concurrent checkSettings completions. Also drops the inline rationale comments per review feedback.
didiergarcia
changed the title
macos-latest runner image no longer ships an iPhone 15 simulator, so xcodebuild fails destination lookup before compiling. Pin to iPhone 17 and let Xcode pick whichever iOS version the image has installed.
Remove the updateQueue serialization introduced earlier in this branch. The underlying race lives in analytics-swift Mediator and is addressed there directly. Once consumers upgrade to the analytics-swift version with that fix, Mediator protects concurrent add(plugin:) calls without any additional coordination from this plugin. Keep the installBlockerIfNeeded(on:type:make:) helper, which closes a pre-existing check-then-insert TOCTOU in update(settings:type:) where overlapping settings fetches could each pass the find() == nil guard and install duplicate blockers.
didiergarcia
changed the title
bsneed
approved these changes
May 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Small refactor of
ConsentManager.update(settings:type:): collapse the per-destinationfind(pluginType:) == nilcheck and the subsequentdestination.add(plugin:)into a singleinstallBlockerIfNeeded(on:type:make:)helper. Same behavior, but the guard and the insert are now co-located rather than repeated forSegmentConsentBlockerandConsentBlocker.Also removed iPhone 15 target that is no longer available.
Scope
Sources/SegmentConsent/Manager.swiftonly (+9/-13).SegmentConsent-Testspaths unaffected.Test plan
swift buildsucceeds.SegmentConsent-Testsstill passes (19 tests across ConsentBlockerTests, ConsentNotEnabledAtSegment, DestinationsMultipleCategoriesTests, NoUnmappedDestinationsTests, UnmappedDestinationsTests).