Skip to content

iOS Security Stack 2/5: Concurrency Locks#33030

Closed
mbelinky wants to merge 11 commits intostack/ios-sec-01-keychain-migrationsfrom
stack/ios-sec-02-concurrency-locks
Closed

iOS Security Stack 2/5: Concurrency Locks#33030
mbelinky wants to merge 11 commits intostack/ios-sec-01-keychain-migrationsfrom
stack/ios-sec-02-concurrency-locks

Conversation

@mbelinky
Copy link
Copy Markdown
Contributor

@mbelinky mbelinky commented Mar 3, 2026

Part 2/5 of iOS security stack (split from #31979).

Scope

  • Fix data races in iOS TLS probe + camera delegates using OSAllocatedUnfairLock

Files

  • apps/ios/Sources/Camera/CameraController.swift
  • apps/ios/Sources/Gateway/GatewayConnectionController.swift

Stack

  • Base: Part 1/5 (stack/ios-sec-01-keychain-migrations)
  • Next PR: Part 3/5 (stack/ios-sec-03-security-guards)

@openclaw-barnacle openclaw-barnacle bot added app: ios App: ios size: S maintainer Maintainer-authored PR labels Mar 3, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 3, 2026

Greptile Summary

This PR replaces unsynchronized mutable state in PhotoCaptureDelegate, MovieFileDelegate, and GatewayTLSFingerprintProbe with OSAllocatedUnfairLock-protected state, correctly eliminating data races that existed when AVFoundation/URLSession delegate callbacks fired concurrently with timeout handlers or each other.

Key changes:

  • PhotoCaptureDelegate and MovieFileDelegate: private var didResume = false becomes private let resumed = OSAllocatedUnfairLock(initialState: false). The check-and-set is now atomic via withLock, preventing double-resumption of a CheckedContinuation.
  • GatewayTLSFingerprintProbe: three separate unguarded mutable vars (didFinish, session, task) are consolidated into a ProbeState struct protected by a single OSAllocatedUnfairLock. The finish() method now atomically checks the flag and drains the session/task references under the lock before cancelling/invalidating outside it — a clean improvement over the previous objc_sync_enter/objc_sync_exit approach that also avoids holding the lock during I/O teardown.

The import Observation already in the file implies a deployment target ≥ iOS 17, so OSAllocatedUnfairLock (available from iOS 16) is available without any #available guard.

Confidence Score: 5/5

  • This PR is safe to merge — changes are narrowly scoped, mechanically correct, and strictly improve thread safety without altering observable behavior.
  • The lock usage follows the correct check-and-set pattern throughout. OSAllocatedUnfairLock is appropriate for the use-case (short, non-blocking critical sections), no re-entrant lock acquisitions exist, and clearing session/task inside the lock before cancelling outside it is both correct and an improvement over the prior code. Deployment target is confirmed ≥ iOS 17 via existing import Observation, so no availability concern.
  • No files require special attention.

Last reviewed commit: 2d676d6

@mbelinky mbelinky force-pushed the stack/ios-sec-02-concurrency-locks branch 2 times, most recently from 4e15a3d to b69e706 Compare March 3, 2026 15:25
@mbelinky mbelinky force-pushed the stack/ios-sec-01-keychain-migrations branch from 92e3ee0 to 8d538be Compare March 3, 2026 15:42
@mbelinky mbelinky force-pushed the stack/ios-sec-02-concurrency-locks branch from b69e706 to 0727b96 Compare March 3, 2026 15:43
@mbelinky mbelinky force-pushed the stack/ios-sec-02-concurrency-locks branch from 0727b96 to de6a2c4 Compare March 3, 2026 15:56
@mbelinky mbelinky force-pushed the stack/ios-sec-01-keychain-migrations branch from 0b88928 to 8e98cde Compare March 3, 2026 16:10
@mbelinky mbelinky force-pushed the stack/ios-sec-02-concurrency-locks branch from de6a2c4 to d5b5846 Compare March 3, 2026 16:10
@mbelinky mbelinky force-pushed the stack/ios-sec-01-keychain-migrations branch from 8e98cde to b37f3a2 Compare March 3, 2026 16:13
@mbelinky mbelinky force-pushed the stack/ios-sec-02-concurrency-locks branch from d5b5846 to dd85dc7 Compare March 3, 2026 16:13
@mbelinky mbelinky force-pushed the stack/ios-sec-01-keychain-migrations branch from b37f3a2 to da2f8f6 Compare March 3, 2026 16:14
@mbelinky mbelinky deleted the branch stack/ios-sec-01-keychain-migrations March 3, 2026 16:15
@mbelinky mbelinky closed this Mar 3, 2026
@mbelinky
Copy link
Copy Markdown
Contributor Author

mbelinky commented Mar 3, 2026

Superseded by #33241 after this PR auto-closed when its base branch was deleted. Equivalent concurrency-lock scope was landed in #33241 (merge commit 6df57d9).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: ios App: ios maintainer Maintainer-authored PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants