Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added fixes and defensive coding for a number of crashes #681

Merged
merged 1 commit into from Aug 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Session/Onboarding/LandingVC.swift
Expand Up @@ -65,7 +65,7 @@ final class LandingVC: BaseVC {
linkButtonContainer.set(.height, to: Values.onboardingButtonBottomOffset)
linkButtonContainer.addSubview(linkButton)
linkButton.center(.horizontal, in: linkButtonContainer)
let isIPhoneX = (UIApplication.shared.keyWindow!.safeAreaInsets.bottom > 0)
let isIPhoneX = ((UIApplication.shared.keyWindow?.safeAreaInsets.bottom ?? 0) > 0)
linkButton.centerYAnchor.constraint(equalTo: linkButtonContainer.centerYAnchor, constant: isIPhoneX ? -4 : 0).isActive = true
// Button stack view
let buttonStackView = UIStackView(arrangedSubviews: [ registerButton, restoreButton ])
Expand Down
10 changes: 5 additions & 5 deletions SessionMessagingKit/Open Groups/OpenGroupManager.swift
Expand Up @@ -19,9 +19,9 @@ public protocol OGMCacheType {
var hasPerformedInitialPoll: [String: Bool] { get set }
var timeSinceLastPoll: [String: TimeInterval] { get set }

func getTimeSinceLastOpen(using dependencies: Dependencies) -> TimeInterval

var pendingChanges: [OpenGroupAPI.PendingChange] { get set }

func getTimeSinceLastOpen(using dependencies: Dependencies) -> TimeInterval
}

// MARK: - OpenGroupManager
Expand Down Expand Up @@ -1099,10 +1099,10 @@ public final class OpenGroupManager: NSObject {

extension OpenGroupManager {
public class OGMDependencies: SMKDependencies {
internal var _mutableCache: Atomic<OGMCacheType>?
internal var _mutableCache: Atomic<Atomic<OGMCacheType>?>
public var mutableCache: Atomic<OGMCacheType> {
get { Dependencies.getValueSettingIfNull(&_mutableCache) { OpenGroupManager.shared.mutableCache } }
set { _mutableCache = newValue }
set { _mutableCache.mutate { $0 = newValue } }
}

public var cache: OGMCacheType { return mutableCache.wrappedValue }
Expand All @@ -1123,7 +1123,7 @@ extension OpenGroupManager {
standardUserDefaults: UserDefaultsType? = nil,
date: Date? = nil
) {
_mutableCache = cache
_mutableCache = Atomic(cache)

super.init(
onionApi: onionApi,
Expand Down
Expand Up @@ -89,7 +89,7 @@ public final class Poller {

private func pollNextSnode(seal: Resolver<Void>) {
let userPublicKey = getUserHexEncodedPublicKey()
let swarm = SnodeAPI.swarmCache[userPublicKey] ?? []
let swarm = SnodeAPI.swarmCache.wrappedValue[userPublicKey] ?? []
let unusedSnodes = swarm.subtracting(usedSnodes)

guard !unusedSnodes.isEmpty else {
Expand Down
54 changes: 27 additions & 27 deletions SessionMessagingKit/Utilities/SMKDependencies.swift
Expand Up @@ -6,58 +6,58 @@ import SessionSnodeKit
import SessionUtilitiesKit

public class SMKDependencies: Dependencies {
internal var _onionApi: OnionRequestAPIType.Type?
internal var _onionApi: Atomic<OnionRequestAPIType.Type?>
public var onionApi: OnionRequestAPIType.Type {
get { Dependencies.getValueSettingIfNull(&_onionApi) { OnionRequestAPI.self } }
set { _onionApi = newValue }
set { _onionApi.mutate { $0 = newValue } }
}

internal var _sodium: SodiumType?
internal var _sodium: Atomic<SodiumType?>
public var sodium: SodiumType {
get { Dependencies.getValueSettingIfNull(&_sodium) { Sodium() } }
set { _sodium = newValue }
set { _sodium.mutate { $0 = newValue } }
}

internal var _box: BoxType?
internal var _box: Atomic<BoxType?>
public var box: BoxType {
get { Dependencies.getValueSettingIfNull(&_box) { sodium.getBox() } }
set { _box = newValue }
set { _box.mutate { $0 = newValue } }
}

internal var _genericHash: GenericHashType?
internal var _genericHash: Atomic<GenericHashType?>
public var genericHash: GenericHashType {
get { Dependencies.getValueSettingIfNull(&_genericHash) { sodium.getGenericHash() } }
set { _genericHash = newValue }
set { _genericHash.mutate { $0 = newValue } }
}

internal var _sign: SignType?
internal var _sign: Atomic<SignType?>
public var sign: SignType {
get { Dependencies.getValueSettingIfNull(&_sign) { sodium.getSign() } }
set { _sign = newValue }
set { _sign.mutate { $0 = newValue } }
}

internal var _aeadXChaCha20Poly1305Ietf: AeadXChaCha20Poly1305IetfType?
internal var _aeadXChaCha20Poly1305Ietf: Atomic<AeadXChaCha20Poly1305IetfType?>
public var aeadXChaCha20Poly1305Ietf: AeadXChaCha20Poly1305IetfType {
get { Dependencies.getValueSettingIfNull(&_aeadXChaCha20Poly1305Ietf) { sodium.getAeadXChaCha20Poly1305Ietf() } }
set { _aeadXChaCha20Poly1305Ietf = newValue }
set { _aeadXChaCha20Poly1305Ietf.mutate { $0 = newValue } }
}

internal var _ed25519: Ed25519Type?
internal var _ed25519: Atomic<Ed25519Type?>
public var ed25519: Ed25519Type {
get { Dependencies.getValueSettingIfNull(&_ed25519) { Ed25519Wrapper() } }
set { _ed25519 = newValue }
set { _ed25519.mutate { $0 = newValue } }
}

internal var _nonceGenerator16: NonceGenerator16ByteType?
internal var _nonceGenerator16: Atomic<NonceGenerator16ByteType?>
public var nonceGenerator16: NonceGenerator16ByteType {
get { Dependencies.getValueSettingIfNull(&_nonceGenerator16) { OpenGroupAPI.NonceGenerator16Byte() } }
set { _nonceGenerator16 = newValue }
set { _nonceGenerator16.mutate { $0 = newValue } }
}

internal var _nonceGenerator24: NonceGenerator24ByteType?
internal var _nonceGenerator24: Atomic<NonceGenerator24ByteType?>
public var nonceGenerator24: NonceGenerator24ByteType {
get { Dependencies.getValueSettingIfNull(&_nonceGenerator24) { OpenGroupAPI.NonceGenerator24Byte() } }
set { _nonceGenerator24 = newValue }
set { _nonceGenerator24.mutate { $0 = newValue } }
}

// MARK: - Initialization
Expand All @@ -77,15 +77,15 @@ public class SMKDependencies: Dependencies {
standardUserDefaults: UserDefaultsType? = nil,
date: Date? = nil
) {
_onionApi = onionApi
_sodium = sodium
_box = box
_genericHash = genericHash
_sign = sign
_aeadXChaCha20Poly1305Ietf = aeadXChaCha20Poly1305Ietf
_ed25519 = ed25519
_nonceGenerator16 = nonceGenerator16
_nonceGenerator24 = nonceGenerator24
_onionApi = Atomic(onionApi)
_sodium = Atomic(sodium)
_box = Atomic(box)
_genericHash = Atomic(genericHash)
_sign = Atomic(sign)
_aeadXChaCha20Poly1305Ietf = Atomic(aeadXChaCha20Poly1305Ietf)
_ed25519 = Atomic(ed25519)
_nonceGenerator16 = Atomic(nonceGenerator16)
_nonceGenerator24 = Atomic(nonceGenerator24)

super.init(
generalCache: generalCache,
Expand Down
6 changes: 4 additions & 2 deletions SessionMessagingKitTests/Open Groups/OpenGroupAPISpec.swift
Expand Up @@ -1243,7 +1243,8 @@ class OpenGroupAPISpec: QuickSpec {
whisperMods: false,
whisperTo: nil,
base64EncodedData: nil,
base64EncodedSignature: nil
base64EncodedSignature: nil,
reactions: nil
)

override class var mockResponse: Data? { return try! JSONEncoder().encode(data) }
Expand Down Expand Up @@ -1612,7 +1613,8 @@ class OpenGroupAPISpec: QuickSpec {
whisperMods: false,
whisperTo: nil,
base64EncodedData: nil,
base64EncodedSignature: nil
base64EncodedSignature: nil,
reactions: nil
)

override class var mockResponse: Data? { return try! JSONEncoder().encode(data) }
Expand Down
22 changes: 15 additions & 7 deletions SessionMessagingKitTests/Open Groups/OpenGroupManagerSpec.swift
Expand Up @@ -204,7 +204,8 @@ class OpenGroupManagerSpec: QuickSpec {
"AAAAAAAAAAAAAAAAAAAAA",
"AA"
].joined(),
base64EncodedSignature: nil
base64EncodedSignature: nil,
reactions: nil
)
testDirectMessage = OpenGroupAPI.DirectMessage(
id: 128,
Expand All @@ -229,6 +230,7 @@ class OpenGroupManagerSpec: QuickSpec {
try testOpenGroup.insert(db)
try Capability(openGroupServer: testOpenGroup.server, variant: .sogs, isMissing: false).insert(db)
}
mockOGMCache.when { $0.pendingChanges }.thenReturn([])
mockGeneralCache.when { $0.encodedPublicKey }.thenReturn("05\(TestConstants.publicKey)")
mockGenericHash.when { $0.hash(message: anyArray(), outputLength: any()) }.thenReturn([])
mockSodium
Expand Down Expand Up @@ -2115,7 +2117,8 @@ class OpenGroupManagerSpec: QuickSpec {
whisperMods: false,
whisperTo: nil,
base64EncodedData: nil,
base64EncodedSignature: nil
base64EncodedSignature: nil,
reactions: nil
)
],
for: "testRoom",
Expand Down Expand Up @@ -2175,7 +2178,8 @@ class OpenGroupManagerSpec: QuickSpec {
whisperMods: false,
whisperTo: nil,
base64EncodedData: Data([1, 2, 3]).base64EncodedString(),
base64EncodedSignature: nil
base64EncodedSignature: nil,
reactions: nil
)
],
for: "testRoom",
Expand Down Expand Up @@ -2207,7 +2211,8 @@ class OpenGroupManagerSpec: QuickSpec {
whisperMods: false,
whisperTo: nil,
base64EncodedData: Data([1, 2, 3]).base64EncodedString(),
base64EncodedSignature: nil
base64EncodedSignature: nil,
reactions: nil
)
],
for: "testRoom",
Expand Down Expand Up @@ -2249,7 +2254,8 @@ class OpenGroupManagerSpec: QuickSpec {
whisperMods: false,
whisperTo: nil,
base64EncodedData: Data([1, 2, 3]).base64EncodedString(),
base64EncodedSignature: nil
base64EncodedSignature: nil,
reactions: nil
),
testMessage,
],
Expand Down Expand Up @@ -2287,7 +2293,8 @@ class OpenGroupManagerSpec: QuickSpec {
whisperMods: false,
whisperTo: nil,
base64EncodedData: nil,
base64EncodedSignature: nil
base64EncodedSignature: nil,
reactions: nil
)
],
for: "testRoom",
Expand Down Expand Up @@ -2315,7 +2322,8 @@ class OpenGroupManagerSpec: QuickSpec {
whisperMods: false,
whisperTo: nil,
base64EncodedData: nil,
base64EncodedSignature: nil
base64EncodedSignature: nil,
reactions: nil
)
],
for: "testRoom",
Expand Down
26 changes: 13 additions & 13 deletions SessionMessagingKitTests/_TestUtilities/DependencyExtensions.swift
Expand Up @@ -23,19 +23,19 @@ extension SMKDependencies {
date: Date? = nil
) -> SMKDependencies {
return SMKDependencies(
onionApi: (onionApi ?? self._onionApi),
generalCache: (generalCache ?? self._generalCache),
storage: (storage ?? self._storage),
sodium: (sodium ?? self._sodium),
box: (box ?? self._box),
genericHash: (genericHash ?? self._genericHash),
sign: (sign ?? self._sign),
aeadXChaCha20Poly1305Ietf: (aeadXChaCha20Poly1305Ietf ?? self._aeadXChaCha20Poly1305Ietf),
ed25519: (ed25519 ?? self._ed25519),
nonceGenerator16: (nonceGenerator16 ?? self._nonceGenerator16),
nonceGenerator24: (nonceGenerator24 ?? self._nonceGenerator24),
standardUserDefaults: (standardUserDefaults ?? self._standardUserDefaults),
date: (date ?? self._date)
onionApi: (onionApi ?? self._onionApi.wrappedValue),
generalCache: (generalCache ?? self._generalCache.wrappedValue),
storage: (storage ?? self._storage.wrappedValue),
sodium: (sodium ?? self._sodium.wrappedValue),
box: (box ?? self._box.wrappedValue),
genericHash: (genericHash ?? self._genericHash.wrappedValue),
sign: (sign ?? self._sign.wrappedValue),
aeadXChaCha20Poly1305Ietf: (aeadXChaCha20Poly1305Ietf ?? self._aeadXChaCha20Poly1305Ietf.wrappedValue),
ed25519: (ed25519 ?? self._ed25519.wrappedValue),
nonceGenerator16: (nonceGenerator16 ?? self._nonceGenerator16.wrappedValue),
nonceGenerator24: (nonceGenerator24 ?? self._nonceGenerator24.wrappedValue),
standardUserDefaults: (standardUserDefaults ?? self._standardUserDefaults.wrappedValue),
date: (date ?? self._date.wrappedValue)
)
}
}
Expand Up @@ -10,4 +10,9 @@ class MockGeneralCache: Mock<GeneralCacheType>, GeneralCacheType {
get { return accept() as? String }
set { accept(args: [newValue]) }
}

var recentReactionTimestamps: [Int64] {
get { return accept() as! [Int64] }
set { accept(args: [newValue]) }
}
}
5 changes: 5 additions & 0 deletions SessionMessagingKitTests/_TestUtilities/MockOGMCache.swift
Expand Up @@ -37,6 +37,11 @@ class MockOGMCache: Mock<OGMCacheType>, OGMCacheType {
set { accept(args: [newValue]) }
}

var pendingChanges: [OpenGroupAPI.PendingChange] {
get { return accept() as! [OpenGroupAPI.PendingChange] }
set { accept(args: [newValue]) }
}

func getTimeSinceLastOpen(using dependencies: Dependencies) -> TimeInterval {
return accept(args: [dependencies]) as! TimeInterval
}
Expand Down
Expand Up @@ -24,20 +24,20 @@ extension OpenGroupManager.OGMDependencies {
date: Date? = nil
) -> OpenGroupManager.OGMDependencies {
return OpenGroupManager.OGMDependencies(
cache: (cache ?? self._mutableCache),
onionApi: (onionApi ?? self._onionApi),
generalCache: (generalCache ?? self._generalCache),
storage: (storage ?? self._storage),
sodium: (sodium ?? self._sodium),
box: (box ?? self._box),
genericHash: (genericHash ?? self._genericHash),
sign: (sign ?? self._sign),
aeadXChaCha20Poly1305Ietf: (aeadXChaCha20Poly1305Ietf ?? self._aeadXChaCha20Poly1305Ietf),
ed25519: (ed25519 ?? self._ed25519),
nonceGenerator16: (nonceGenerator16 ?? self._nonceGenerator16),
nonceGenerator24: (nonceGenerator24 ?? self._nonceGenerator24),
standardUserDefaults: (standardUserDefaults ?? self._standardUserDefaults),
date: (date ?? self._date)
cache: (cache ?? self._mutableCache.wrappedValue),
onionApi: (onionApi ?? self._onionApi.wrappedValue),
generalCache: (generalCache ?? self._generalCache.wrappedValue),
storage: (storage ?? self._storage.wrappedValue),
sodium: (sodium ?? self._sodium.wrappedValue),
box: (box ?? self._box.wrappedValue),
genericHash: (genericHash ?? self._genericHash.wrappedValue),
sign: (sign ?? self._sign.wrappedValue),
aeadXChaCha20Poly1305Ietf: (aeadXChaCha20Poly1305Ietf ?? self._aeadXChaCha20Poly1305Ietf.wrappedValue),
ed25519: (ed25519 ?? self._ed25519.wrappedValue),
nonceGenerator16: (nonceGenerator16 ?? self._nonceGenerator16.wrappedValue),
nonceGenerator24: (nonceGenerator24 ?? self._nonceGenerator24.wrappedValue),
standardUserDefaults: (standardUserDefaults ?? self._standardUserDefaults.wrappedValue),
date: (date ?? self._date.wrappedValue)
)
}
}
16 changes: 9 additions & 7 deletions SessionSnodeKit/SnodeAPI.swift
Expand Up @@ -24,7 +24,7 @@ public final class SnodeAPI {
/// - Note: Should only be accessed from `Threading.workQueue` to avoid race conditions.
public static var clockOffset: Int64 = 0
/// - Note: Should only be accessed from `Threading.workQueue` to avoid race conditions.
public static var swarmCache: [String: Set<Snode>] = [:]
public static var swarmCache: Atomic<[String: Set<Snode>]> = Atomic([:])

// MARK: - Namespaces

Expand Down Expand Up @@ -96,18 +96,20 @@ public final class SnodeAPI {
private static func loadSwarmIfNeeded(for publicKey: String) {
guard !loadedSwarms.contains(publicKey) else { return }

Storage.shared.read { db in
swarmCache[publicKey] = ((try? Snode.fetchSet(db, publicKey: publicKey)) ?? [])
}
let updatedCacheForKey: Set<Snode> = Storage.shared
.read { db in try Snode.fetchSet(db, publicKey: publicKey) }
.defaulting(to: [])

swarmCache.mutate { $0[publicKey] = updatedCacheForKey }
loadedSwarms.insert(publicKey)
}

private static func setSwarm(to newValue: Set<Snode>, for publicKey: String, persist: Bool = true) {
#if DEBUG
dispatchPrecondition(condition: .onQueue(Threading.workQueue))
#endif
swarmCache[publicKey] = newValue
swarmCache.mutate { $0[publicKey] = newValue }

guard persist else { return }

Storage.shared.write { db in
Expand All @@ -119,7 +121,7 @@ public final class SnodeAPI {
#if DEBUG
dispatchPrecondition(condition: .onQueue(Threading.workQueue))
#endif
let swarmOrNil = swarmCache[publicKey]
let swarmOrNil = swarmCache.wrappedValue[publicKey]
guard var swarm = swarmOrNil, let index = swarm.firstIndex(of: snode) else { return }
swarm.remove(at: index)
setSwarm(to: swarm, for: publicKey)
Expand Down Expand Up @@ -460,7 +462,7 @@ public final class SnodeAPI {
public static func getSwarm(for publicKey: String) -> Promise<Set<Snode>> {
loadSwarmIfNeeded(for: publicKey)

if let cachedSwarm = swarmCache[publicKey], cachedSwarm.count >= minSwarmSnodeCount {
if let cachedSwarm = swarmCache.wrappedValue[publicKey], cachedSwarm.count >= minSwarmSnodeCount {
return Promise<Set<Snode>> { $0.fulfill(cachedSwarm) }
}

Expand Down