perf: replace DispatchQueue.sync with NSLock in RegexManager to prevent UI freezes

[EugenePetlitskiy]

Description

This PR optimizes the thread synchronization mechanism in RegexManager by replacing the serial DispatchQueue with an NSLock to manage access to the regularExpressionPool dictionary.

Motivation and Context

Currently, RegexManager uses a serial queue (regularExpressionPoolQueue.sync) to protect the regex cache from data races. However, there is no explicit documentation warning consumers against calling methods like phoneDataDetectorMatch or regexWithPattern concurrently from multiple threads. As a result, consumers naturally use the library across various concurrent queues.

When accessed heavily in a multi-threaded environment, the DispatchQueue.sync approach becomes a significant bottleneck. The overhead of GCD context switching, queue management, and potential priority inversion makes it an uneconomical choice for simple, high-frequency dictionary read/write operations.

Switching to NSLock provides a much lower-level, lightweight locking mechanism. It guarantees the necessary thread safety (preventing EXC_BAD_ACCESS during concurrent dictionary mutations) while drastically reducing overhead, lock contention time, and improving overall throughput for concurrent callers.

How Has This Been Tested?

• Verified that no crashes occur during heavy concurrent reads/writes to the pool from multiple background threads.
• Compared performance under concurrent load, confirming a reduction in thread waiting time.
• Existing unit tests pass successfully.

Types of changes

• Performance improvement (non-breaking change that optimizes existing execution)
• Refactoring (improving internal architecture without changing external behavior)

[bguidolim]

PR Review Summary

Overall Assessment

The motivation is sound and well-documented — NSLock is genuinely lighter weight than DispatchQueue.sync for simple mutex patterns. Independent benchmarks confirm DispatchQueue.sync is 7–8× slower than locks for read operations and 3–4× slower for writes. Apple's own DTS engineer Quinn "The Eskimo" recommends NSLock as the go-to lock until OSAllocatedUnfairLock (iOS 16+) can be relied upon.

Given the project's iOS 12+ deployment target, NSLock is the right choice. 👍

---

Critical: Missing defer { unlock() } — scope safety regression

The original DispatchQueue.sync { } pattern was inherently scope-safe — the closure guaranteed lock release on exit. The new manual lock()/unlock() pattern loses this guarantee.

If any code is ever added between lock() and unlock() that throws, returns early, or traps, the lock will be held permanently — deadlocking all subsequent phone number operations across all threads.

Recommended fix — add defer after every lock() call:

func regexWithPattern(_ pattern: String) throws -> NSRegularExpression {
    regularExpressionLock.lock()
    defer { regularExpressionLock.unlock() }
    let cached = regularExpressionPool[pattern]

    if let cached {
        return cached
    }

    do {
        let regex = try NSRegularExpression(pattern: pattern, options: .caseInsensitive)
        regularExpressionPool[pattern] = regex
        return regex
    } catch {
        throw PhoneNumberError.generalError
    }
}

Note: With defer, we can actually simplify to a single lock region — the lock is released on any return or throw, so there's no risk of holding it during the NSRegularExpression init. This also eliminates the pre-existing TOCTOU race (two threads missing the cache simultaneously and compiling the same regex twice).

Minor: Style nit

Extra blank line at the top of regexWithPattern is inconsistent with the rest of the file.

---

Summary

Issue	Severity	Action
Missing defer { unlock() } — deadlock risk	Critical	Must fix before merge
TOCTOU race on concurrent cache miss	Low	Pre-existing, fixed for free with single lock region
Stray blank line	Trivial	Clean up

The performance improvement is valid and welcome. Just needs the defer guard to maintain the same safety level as the original code. Thanks for the contribution!

[EugenePetlitskiy]

Critical: Missing defer { unlock() } — scope safety regression

The original DispatchQueue.sync { } pattern was inherently scope-safe — the closure guaranteed lock release on exit. The new manual lock()/unlock() pattern loses this guarantee.

If any code is ever added between lock() and unlock() that throws, returns early, or traps, the lock will be held permanently — deadlocking all subsequent phone number operations across all threads.

Recommended fix — add defer after every lock() call:

Thanks for the detailed review and the great catch with defer! I've pushed the requested changes.