Fixed remaining issues, cleaned up memory & logic

# Conflicts:
#	Session.xcodeproj/project.pbxproj
This commit is contained in:
Morgan Pretty 2022-11-29 09:48:55 +11:00
parent a1e09d830f
commit d03d2ce8ab
5 changed files with 86 additions and 46 deletions

View File

@ -254,7 +254,7 @@
B8DE1FB626C22FCB0079C9CE /* CallMessage.swift in Sources */ = {isa = PBXBuildFile; fileRef = B8DE1FB526C22FCB0079C9CE /* CallMessage.swift */; };
B8EB20EE2640F28000773E52 /* VisibleMessage+OpenGroupInvitation.swift in Sources */ = {isa = PBXBuildFile; fileRef = B8EB20ED2640F28000773E52 /* VisibleMessage+OpenGroupInvitation.swift */; };
B8EB20F02640F7F000773E52 /* OpenGroupInvitationView.swift in Sources */ = {isa = PBXBuildFile; fileRef = B8EB20EF2640F7F000773E52 /* OpenGroupInvitationView.swift */; };
B8F5F58325EC94A6003BF8D4 /* Collection+Subscripting.swift in Sources */ = {isa = PBXBuildFile; fileRef = B8F5F58225EC94A6003BF8D4 /* Collection+Subscripting.swift */; };
B8F5F58325EC94A6003BF8D4 /* Collection+Utilities.swift in Sources */ = {isa = PBXBuildFile; fileRef = B8F5F58225EC94A6003BF8D4 /* Collection+Utilities.swift */; };
B8F5F60325EDE16F003BF8D4 /* DataExtractionNotification.swift in Sources */ = {isa = PBXBuildFile; fileRef = B8F5F60225EDE16F003BF8D4 /* DataExtractionNotification.swift */; };
B8F5F71A25F1B35C003BF8D4 /* MediaPlaceholderView.swift in Sources */ = {isa = PBXBuildFile; fileRef = B8F5F71925F1B35C003BF8D4 /* MediaPlaceholderView.swift */; };
B8FF8DAE25C0D00F004D1F22 /* SessionMessagingKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = C3C2A6F025539DE700C340D1 /* SessionMessagingKit.framework */; };
@ -1371,7 +1371,7 @@
B8EB20E6263F7E4B00773E52 /* sk */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = sk; path = sk.lproj/Localizable.strings; sourceTree = "<group>"; };
B8EB20ED2640F28000773E52 /* VisibleMessage+OpenGroupInvitation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "VisibleMessage+OpenGroupInvitation.swift"; sourceTree = "<group>"; };
B8EB20EF2640F7F000773E52 /* OpenGroupInvitationView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OpenGroupInvitationView.swift; sourceTree = "<group>"; };
B8F5F58225EC94A6003BF8D4 /* Collection+Subscripting.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Collection+Subscripting.swift"; sourceTree = "<group>"; };
B8F5F58225EC94A6003BF8D4 /* Collection+Utilities.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Collection+Utilities.swift"; sourceTree = "<group>"; };
B8F5F60225EDE16F003BF8D4 /* DataExtractionNotification.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataExtractionNotification.swift; sourceTree = "<group>"; };
B8F5F71925F1B35C003BF8D4 /* MediaPlaceholderView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MediaPlaceholderView.swift; sourceTree = "<group>"; };
B8FF8E6125C10DA5004D1F22 /* GeoLite2-Country-Blocks-IPv4 */ = {isa = PBXFileReference; lastKnownFileType = file.bplist; name = "GeoLite2-Country-Blocks-IPv4"; path = "Countries/GeoLite2-Country-Blocks-IPv4"; sourceTree = "<group>"; };
@ -2577,6 +2577,7 @@
B8F5F58225EC94A6003BF8D4 /* Collection+Subscripting.swift */,
FDFD645A27F26D4600808CA1 /* Data+Utilities.swift */,
FDC6D75F2862B3F600B04575 /* Dependencies.swift */,
FD26FA5D291CAFF9005801D8 /* Data+Utilities.swift */,
C3C2A5D52553860A00C340D1 /* Dictionary+Utilities.swift */,
B87EF18026377A1D00124B3C /* Features.swift */,
B8BC00BF257D90E30032E807 /* General.swift */,
@ -5445,7 +5446,7 @@
FD52090028AF6153006098F6 /* OWSBackgroundTask.m in Sources */,
C32C5DDB256DD9FF003C73A2 /* ContentProxy.swift in Sources */,
C3A71F892558BA9F0043A11F /* Mnemonic.swift in Sources */,
B8F5F58325EC94A6003BF8D4 /* Collection+Subscripting.swift in Sources */,
B8F5F58325EC94A6003BF8D4 /* Collection+Utilities.swift in Sources */,
C33FDEF8255A656D00E217F9 /* Promise+Delaying.swift in Sources */,
7BD477A827EC39F5004E2822 /* Atomic.swift in Sources */,
B8BC00C0257D90E30032E807 /* General.swift in Sources */,

View File

@ -2,6 +2,7 @@
import Foundation
import SessionUtil
import SessionUtilitiesKit
import Quick
import Nimble
@ -11,11 +12,12 @@ class ConfigUserProfileSpec: QuickSpec {
// MARK: - Spec
override func spec() {
it("behaves correctly") {
it("generates configs correctly") {
// Initialize a brand new, empty config because we have no dump data to deal with.
let error: UnsafeMutablePointer<CChar>? = nil
var conf: UnsafeMutablePointer<config_object>? = nil
expect(user_profile_init(&conf, nil, 0, error)).to(equal(0))
error?.deallocate()
// We don't need to push anything, since this is an empty config
expect(config_needs_push(conf)).to(beFalse())
@ -23,7 +25,7 @@ class ConfigUserProfileSpec: QuickSpec {
expect(config_needs_dump(conf)).to(beFalse())
// Since it's empty there shouldn't be a name.
let namePtr = user_profile_get_name(conf)
let namePtr: UnsafePointer<CChar>? = user_profile_get_name(conf)
expect(namePtr).to(beNil())
var toPush: UnsafeMutablePointer<CChar>? = nil
@ -33,14 +35,14 @@ class ConfigUserProfileSpec: QuickSpec {
let seqno: Int64 = config_push(conf, &toPush, &toPushLen)
expect(toPush).toNot(beNil())
expect(seqno).to(equal(0))
expect(String(cString: toPush!)).to(equal("d1:#i0e1:&de1:<le1:=dee"))
expect(String(pointer: toPush, length: toPushLen)).to(equal("d1:#i0e1:&de1:<le1:=dee"))
toPush?.deallocate()
// This should also be unset:
let pic = user_profile_get_pic(conf)
let pic: user_profile_pic = user_profile_get_pic(conf)
expect(pic.url).to(beNil())
expect(pic.key).to(beNil())
expect(pic.keylen).to(equal(0))
// Now let's go set a profile name and picture:
expect(user_profile_set_name(conf, "Kallie")).to(equal(0))
@ -62,16 +64,16 @@ class ConfigUserProfileSpec: QuickSpec {
expect(user_profile_set_pic(conf, p)).to(equal(0))
// Retrieve them just to make sure they set properly:
let namePtr2 = user_profile_get_name(conf)
let namePtr2: UnsafePointer<CChar>? = user_profile_get_name(conf)
expect(namePtr2).toNot(beNil())
expect(String(cString: namePtr2!)).to(equal("Kallie"))
let pic2 = user_profile_get_pic(conf);
let pic2: user_profile_pic = user_profile_get_pic(conf);
expect(pic2.url).toNot(beNil())
expect(pic2.key).toNot(beNil())
expect(pic2.keylen).to(equal(6))
expect(String(cString: pic2.url!)).to(equal("http://example.org/omg-pic-123.bmp"))
expect(String(cString: pic2.key!)).to(equal("secret"))
expect(String(pointer: pic2.key, length: pic2.keylen)).to(equal("secret"))
// Since we've made changes, we should need to push new config to the swarm, *and* should need
// to dump the updated state:
@ -80,7 +82,7 @@ class ConfigUserProfileSpec: QuickSpec {
var toPush2: UnsafeMutablePointer<CChar>? = nil
var toPush2Len: Int = 0
let seqno2 = config_push(conf, &toPush2, &toPush2Len);
let seqno2: Int64 = config_push(conf, &toPush2, &toPush2Len);
// incremented since we made changes (this only increments once between
// dumps; even though we changed two fields here).
expect(seqno2).to(equal(1))
@ -121,9 +123,9 @@ class ConfigUserProfileSpec: QuickSpec {
]
.flatMap { $0 }
expect(String(cString: toPush2!))
// Need to null-terminate the string or it'll crash
.to(equal(String(cString: expPush1.appending(CChar()))))
expect(String(pointer: toPush2, length: toPush2Len, encoding: .ascii))
.to(equal(String(pointer: expPush1, length: expPush1.count, encoding: .ascii)))
toPush2?.deallocate()
// We haven't dumped, so still need to dump:
expect(config_needs_dump(conf)).to(beTrue())
@ -155,8 +157,9 @@ class ConfigUserProfileSpec: QuickSpec {
.map { CChar(bitPattern: $0) }
]
.flatMap { $0 }
expect(String(cString: dump1!))
.to(equal(String(cString: expDump1.appending(CChar()))))// Need to null-terminate the string or it'll crash
expect(String(pointer: dump1, length: dump1Len, encoding: .ascii))
.to(equal(String(pointer: expDump1, length: expDump1.count, encoding: .ascii)))
dump1?.deallocate()
// So now imagine we got back confirmation from the swarm that the push has been stored:
config_confirm_pushed(conf, seqno2)
@ -167,7 +170,7 @@ class ConfigUserProfileSpec: QuickSpec {
var dump2: UnsafeMutablePointer<CChar>? = nil
var dump2Len: Int = 0
config_dump(conf, &dump2, &dump2Len)
dump2?.deallocate()
expect(config_needs_dump(conf)).to(beFalse())
// Now we're going to set up a second, competing config object (in the real world this would be
@ -178,18 +181,14 @@ class ConfigUserProfileSpec: QuickSpec {
var conf2: UnsafeMutablePointer<config_object>? = nil
expect(user_profile_init(&conf2, nil, 0, error2)).to(equal(0))
expect(config_needs_dump(conf2)).to(beFalse())
error2?.deallocate()
// Now imagine we just pulled down the `exp_push1` string from the swarm; we merge it into
// conf2:
let mergeData: [[CChar]] = [expPush1]
var mergeDataPtr = mergeData.map { value in
let cStringCopy = UnsafeMutableBufferPointer<CChar>.allocate(capacity: value.count)
_ = cStringCopy.initialize(from: value)
let ptr = UnsafePointer(cStringCopy.baseAddress)
return UnsafePointer(cStringCopy.baseAddress)
}
var mergeData: [UnsafePointer<CChar>?] = [expPush1].unsafeCopy()
var mergeSize: [Int] = [expPush1.count]
config_merge(conf2, &mergeDataPtr, &mergeSize, 1)
config_merge(conf2, &mergeData, &mergeSize, 1)
mergeData.forEach { $0?.deallocate() }
// Our state has changed, so we need to dump:
expect(config_needs_dump(conf2)).to(beTrue())
@ -197,6 +196,7 @@ class ConfigUserProfileSpec: QuickSpec {
var dump3Len: Int = 0
config_dump(conf2, &dump3, &dump3Len)
// (store in db)
dump3?.deallocate()
expect(config_needs_dump(conf2)).to(beFalse())
// We *don't* need to push: even though we updated, all we did is update to the merged data (and
@ -232,12 +232,12 @@ class ConfigUserProfileSpec: QuickSpec {
expect(config_needs_push(conf2)).to(beTrue())
var toPush3: UnsafeMutablePointer<CChar>? = nil
var toPush3Len: Int = 0
let seqno3 = config_push(conf, &toPush3, &toPush3Len)
let seqno3: Int64 = config_push(conf, &toPush3, &toPush3Len)
expect(seqno3).to(equal(2)) // incremented, since we made a field change
var toPush4: UnsafeMutablePointer<CChar>? = nil
var toPush4Len: Int = 0
let seqno4 = config_push(conf2, &toPush4, &toPush4Len)
let seqno4: Int64 = config_push(conf2, &toPush4, &toPush4Len)
expect(seqno4).to(equal(2)) // incremented, since we made a field change
var dump4: UnsafeMutablePointer<CChar>? = nil
@ -247,29 +247,34 @@ class ConfigUserProfileSpec: QuickSpec {
var dump5Len: Int = 0
config_dump(conf2, &dump5, &dump5Len);
// (store in db)
dump4?.deallocate()
dump5?.deallocate()
// Since we set different things, we're going to get back different serialized data to be
// pushed:
expect(String(cString: toPush3!)).toNot(equal(String(cString: toPush4!)))
expect(String(pointer: toPush3, length: toPush3Len, encoding: .ascii))
.toNot(equal(String(pointer: toPush4, length: toPush4Len, encoding: .ascii)))
// Now imagine that each client pushed its `seqno=2` config to the swarm, but then each client
// also fetches new messages and pulls down the other client's `seqno=2` value.
// Feed the new config into each other. (This array could hold multiple configs if we pulled
// down more than one).
var mergeData2Ptr: [UnsafePointer<CChar>?] = [UnsafePointer(toPush3)]
var mergeData2: [UnsafePointer<CChar>?] = [UnsafePointer(toPush3)]
var mergeSize2: [Int] = [toPush3Len]
config_merge(conf2, &mergeData2Ptr, &mergeSize2, 1)
config_merge(conf2, &mergeData2, &mergeSize2, 1)
toPush3?.deallocate()
var mergeData3: [UnsafePointer<CChar>?] = [UnsafePointer(toPush4)]
var mergeSize3: [Int] = [toPush4Len]
config_merge(conf, &mergeData3, &mergeSize3, 1)
toPush4?.deallocate()
// Now after the merge we *will* want to push from both client, since both will have generated a
// merge conflict update (with seqno = 3).
expect(config_needs_push(conf)).to(beTrue())
expect(config_needs_push(conf2)).to(beTrue())
let seqno5 = config_push(conf, &toPush3, &toPush3Len);
let seqno6 = config_push(conf2, &toPush4, &toPush4Len);
let seqno5: Int64 = config_push(conf, &toPush3, &toPush3Len);
let seqno6: Int64 = config_push(conf2, &toPush4, &toPush4Len);
expect(seqno5).to(equal(3))
expect(seqno6).to(equal(3))
@ -282,18 +287,16 @@ class ConfigUserProfileSpec: QuickSpec {
// test).
// Since only one of them set a profile pic there should be no conflict there:
let pic3 = user_profile_get_pic(conf)
let pic3: user_profile_pic = user_profile_get_pic(conf)
expect(pic3.url).toNot(beNil())
expect(String(cString: pic3.url!)).to(equal("http://new.example.com/pic"))
expect(pic3.key).toNot(beNil())
expect(String(data: Data(bytes: pic3.key, count: pic3.keylen), encoding: .utf8))
.to(equal("qwert\0yuio"))
let pic4 = user_profile_get_pic(conf2)
expect(String(pointer: pic3.key, length: pic3.keylen)).to(equal("qwert\0yuio"))
let pic4: user_profile_pic = user_profile_get_pic(conf2)
expect(pic4.url).toNot(beNil())
expect(String(cString: pic4.url!)).to(equal("http://new.example.com/pic"))
expect(pic4.key).toNot(beNil())
expect(String(data: Data(bytes: pic4.key, count: pic4.keylen), encoding: .utf8))
.to(equal("qwert\0yuio"))
expect(String(pointer: pic4.key, length: pic4.keylen)).to(equal("qwert\0yuio"))
config_confirm_pushed(conf, seqno5)
config_confirm_pushed(conf2, seqno6)
@ -305,11 +308,18 @@ class ConfigUserProfileSpec: QuickSpec {
var dump7Len: Int = 0
config_dump(conf2, &dump7, &dump7Len);
// (store in db)
dump6?.deallocate()
dump7?.deallocate()
expect(config_needs_dump(conf)).to(beFalse())
expect(config_needs_dump(conf2)).to(beFalse())
expect(config_needs_push(conf)).to(beFalse())
expect(config_needs_push(conf2)).to(beFalse())
// Wouldn't do this in a normal session but doing it here to properly clean up
// after the test
conf?.deallocate()
conf2?.deallocate()
}
}
}

View File

@ -1,7 +0,0 @@
extension Collection {
public subscript(ifValid index: Index) -> Iterator.Element? {
return self.indices.contains(index) ? self[index] : nil
}
}

View File

@ -0,0 +1,22 @@
// Copyright © 2022 Rangeproof Pty Ltd. All rights reserved.
import Foundation
extension Collection {
public subscript(ifValid index: Index) -> Iterator.Element? {
return self.indices.contains(index) ? self[index] : nil
}
}
public extension Collection where Element == [CChar] {
/// This creates an array of UnsafePointer types to access data of the C strings in memory. This array provides no automated
/// memory management of it's children so after use you are responsible for handling the life cycle of the child elements and
/// need to call `deallocate()` on each child.
func unsafeCopy() -> [UnsafePointer<CChar>?] {
return self.map { value in
let copy = UnsafeMutableBufferPointer<CChar>.allocate(capacity: value.count)
_ = copy.initialize(from: value)
return UnsafePointer(copy.baseAddress)
}
}
}

View File

@ -74,6 +74,20 @@ public extension String {
}
}
// MARK: - Convenience
public extension String {
/// Initialize with an optional pointer and a spoecific length
init?(pointer: UnsafeRawPointer?, length: Int, encoding: String.Encoding = .utf8) {
guard
let pointer: UnsafeRawPointer = pointer,
let result: String = String(data: Data(bytes: pointer, count: length), encoding: encoding)
else { return nil }
self = result
}
}
// MARK: - Formatting
public extension String {