From 13942b8818f707bd3ef82cc53212172098066ab3 Mon Sep 17 00:00:00 2001 From: JayShortway <29483617+JayShortway@users.noreply.github.com> Date: Wed, 8 Jan 2025 14:20:04 +0100 Subject: [PATCH] [EXTERNAL] Lock RateLimiter.shouldProceed() entirely to avoid race conditions (#4635) via @nguyenhuy (#4637) * [EXTERNAL] Lock RateLimiter.shouldProceed() entirely to avoid race conditions (#4635) * Lock RateLimiter.shouldProceed() entirely to avoid race conditions Android does this as well: https://github.com/RevenueCat/purchases-android/blob/eb3c8f4aa1a6993f12d6e2f93cc82d26063101a6/purchases/src/main/kotlin/com/revenuecat/purchases/utils/RateLimiter.kt#L10 * Fix typo in RateLimiterTests' file name * Update Sources/Misc/RateLimiter.swift As suggested by @facumenzella Co-authored-by: Facundo Menzella --------- via @nguyenhuy * Update Sources/Misc/RateLimiter.swift via @nguyenhuy Co-authored-by: Huy Nguyen --------- Co-authored-by: Huy Nguyen --- RevenueCat.xcodeproj/project.pbxproj | 8 +++--- Sources/Misc/RateLimiter.swift | 25 +++++++++++-------- ...iterRests.swift => RateLimiterTests.swift} | 2 +- 3 files changed, 19 insertions(+), 16 deletions(-) rename Tests/UnitTests/Misc/{RateLimiterRests.swift => RateLimiterTests.swift} (97%) diff --git a/RevenueCat.xcodeproj/project.pbxproj b/RevenueCat.xcodeproj/project.pbxproj index c7013ef8df..fdbb30b9fc 100644 --- a/RevenueCat.xcodeproj/project.pbxproj +++ b/RevenueCat.xcodeproj/project.pbxproj @@ -57,7 +57,7 @@ 2C7457882CEDF7C0004ACE52 /* BackgroundStyle.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C7457872CEDF7AC004ACE52 /* BackgroundStyle.swift */; }; 2C74578A2CEE314F004ACE52 /* Background.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C7457892CEE314C004ACE52 /* Background.swift */; }; 2C7F0AD32B8EEB4600381179 /* RateLimiter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C7F0AD22B8EEB4600381179 /* RateLimiter.swift */; }; - 2C7F0AD62B8EEF7B00381179 /* RateLimiterRests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C7F0AD42B8EEF0B00381179 /* RateLimiterRests.swift */; }; + 2C7F0AD62B8EEF7B00381179 /* RateLimiterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C7F0AD42B8EEF0B00381179 /* RateLimiterTests.swift */; }; 2C8EC6DB2CCC23B700D6CCF8 /* Template5Preview.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C8EC6DA2CCC23B700D6CCF8 /* Template5Preview.swift */; }; 2C8EC6DD2CCC7C5B00D6CCF8 /* PackageValidator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C8EC6DC2CCC7C5500D6CCF8 /* PackageValidator.swift */; }; 2C8EC6DF2CCD27A500D6CCF8 /* PartialComponentTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C8EC6DE2CCD27A100D6CCF8 /* PartialComponentTests.swift */; }; @@ -1289,7 +1289,7 @@ 2C7457872CEDF7AC004ACE52 /* BackgroundStyle.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BackgroundStyle.swift; sourceTree = ""; }; 2C7457892CEE314C004ACE52 /* Background.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Background.swift; sourceTree = ""; }; 2C7F0AD22B8EEB4600381179 /* RateLimiter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RateLimiter.swift; sourceTree = ""; }; - 2C7F0AD42B8EEF0B00381179 /* RateLimiterRests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RateLimiterRests.swift; sourceTree = ""; }; + 2C7F0AD42B8EEF0B00381179 /* RateLimiterTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RateLimiterTests.swift; sourceTree = ""; }; 2C8EC6AE2CCBD33E00D6CCF8 /* ButtonComponentTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ButtonComponentTests.swift; sourceTree = ""; }; 2C8EC6DA2CCC23B700D6CCF8 /* Template5Preview.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Template5Preview.swift; sourceTree = ""; }; 2C8EC6DC2CCC7C5500D6CCF8 /* PackageValidator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PackageValidator.swift; sourceTree = ""; }; @@ -3514,7 +3514,7 @@ 5712BE9129241F7900A83F15 /* TimingUtilTests.swift */, 4F8DDB682AAA9189000188F2 /* OperationDispatcherTests.swift */, 4FEF41AC2B4F301800CD699F /* MacAppStoreDetectorTests.swift */, - 2C7F0AD42B8EEF0B00381179 /* RateLimiterRests.swift */, + 2C7F0AD42B8EEF0B00381179 /* RateLimiterTests.swift */, ); path = Misc; sourceTree = ""; @@ -6326,7 +6326,7 @@ 1EFA950D2CDB6B6500CA5951 /* BackendPostRedeemWebPurchaseTests.swift in Sources */, 35C05DC82BC8510000109308 /* DiagnosticsTrackerTests.swift in Sources */, 35C272A12BC4084C005A0CE8 /* MockDiagnosticsTracker.swift in Sources */, - 2C7F0AD62B8EEF7B00381179 /* RateLimiterRests.swift in Sources */, + 2C7F0AD62B8EEF7B00381179 /* RateLimiterTests.swift in Sources */, FDAADFCB2BE2A5BF00BD1659 /* MockAllTransactionsProvider.swift in Sources */, 356E2DE82CD3CF930055AABB /* StoredEventTests.swift in Sources */, 351B513F26D4496000BD2BD7 /* MockIdentityManager.swift in Sources */, diff --git a/Sources/Misc/RateLimiter.swift b/Sources/Misc/RateLimiter.swift index 3627acb388..c3a563f92f 100644 --- a/Sources/Misc/RateLimiter.swift +++ b/Sources/Misc/RateLimiter.swift @@ -13,7 +13,8 @@ import Foundation -internal class RateLimiter { +internal final class RateLimiter: @unchecked Sendable { + private let lock = Lock() private var timestamps: [Date?] private var index: Int = 0 private let maxCallsInclusive: Int @@ -30,17 +31,19 @@ internal class RateLimiter { } func shouldProceed() -> Bool { - let now = Date() - let oldestIndex = (index + 1) % maxCallsInclusive - let oldestTimestamp = timestamps[oldestIndex] + return self.lock.perform { + let now = Date() + let oldestIndex = (index + 1) % maxCallsInclusive + let oldestTimestamp = timestamps[oldestIndex] - // Check if the oldest timestamp is outside the rate limiting period or if it's nil - if let oldestTimestamp = oldestTimestamp, now.timeIntervalSince(oldestTimestamp) <= period { - return false - } else { - timestamps[index] = now - index = oldestIndex - return true + // Check if the oldest timestamp is outside the rate limiting period or if it's nil + if let oldestTimestamp = oldestTimestamp, now.timeIntervalSince(oldestTimestamp) <= period { + return false + } else { + timestamps[index] = now + index = oldestIndex + return true + } } } } diff --git a/Tests/UnitTests/Misc/RateLimiterRests.swift b/Tests/UnitTests/Misc/RateLimiterTests.swift similarity index 97% rename from Tests/UnitTests/Misc/RateLimiterRests.swift rename to Tests/UnitTests/Misc/RateLimiterTests.swift index 8621fc45f8..107b5e6a28 100644 --- a/Tests/UnitTests/Misc/RateLimiterRests.swift +++ b/Tests/UnitTests/Misc/RateLimiterTests.swift @@ -7,7 +7,7 @@ // // https://opensource.org/licenses/MIT // -// RateLimiterRests.swift +// RateLimiterTests.swift // // Created by Josh Holtz on 2/27/24.