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

Release 2.8.6 #357

Merged
merged 4 commits into from
Jan 14, 2025
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Session.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -7683,7 +7683,7 @@
CLANG_WARN__ARC_BRIDGE_CAST_NONARC = YES;
CLANG_WARN__DUPLICATE_METHOD_MATCH = YES;
CODE_SIGN_IDENTITY = "iPhone Developer";
CURRENT_PROJECT_VERSION = 519;
CURRENT_PROJECT_VERSION = 526;
ENABLE_BITCODE = NO;
ENABLE_STRICT_OBJC_MSGSEND = YES;
ENABLE_TESTABILITY = YES;
Expand Down Expand Up @@ -7720,7 +7720,7 @@
GCC_WARN_UNUSED_VARIABLE = YES;
HEADER_SEARCH_PATHS = "";
IPHONEOS_DEPLOYMENT_TARGET = 13.0;
MARKETING_VERSION = 2.8.5;
MARKETING_VERSION = 2.8.6;
ONLY_ACTIVE_ARCH = YES;
OTHER_CFLAGS = (
"-fobjc-arc-exceptions",
Expand Down Expand Up @@ -7762,7 +7762,7 @@
CLANG_WARN__ARC_BRIDGE_CAST_NONARC = YES;
CLANG_WARN__DUPLICATE_METHOD_MATCH = YES;
CODE_SIGN_IDENTITY = "iPhone Distribution";
CURRENT_PROJECT_VERSION = 519;
CURRENT_PROJECT_VERSION = 526;
ENABLE_BITCODE = NO;
ENABLE_STRICT_OBJC_MSGSEND = YES;
GCC_NO_COMMON_BLOCKS = YES;
Expand Down Expand Up @@ -7794,7 +7794,7 @@
GCC_WARN_UNUSED_VARIABLE = YES;
HEADER_SEARCH_PATHS = "";
IPHONEOS_DEPLOYMENT_TARGET = 13.0;
MARKETING_VERSION = 2.8.5;
MARKETING_VERSION = 2.8.6;
ONLY_ACTIVE_ARCH = NO;
OTHER_CFLAGS = (
"-DNS_BLOCK_ASSERTIONS=1",
Expand Down
2 changes: 1 addition & 1 deletion Session/Conversations/ConversationVC.swift
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ final class ConversationVC: BaseVC, LibSessionRespondingViewController, Conversa
// PagedDatabaseObserver won't have them so we need to force a re-fetch of the current
// data to ensure everything is up to date
if didReturnFromBackground {
DispatchQueue.global(qos: .background).async {
DispatchQueue.global(qos: .background).asyncAfter(deadline: .now() + 0.01) {
self?.viewModel.pagedDataObserver?.reload()
}
}
Expand Down
2 changes: 1 addition & 1 deletion Session/Conversations/ConversationViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ public class ConversationViewModel: OWSAudioPlayerDelegate, NavigatableStateHold
)

// Run the initial query on a background thread so we don't block the push transition
DispatchQueue.global(qos: .userInitiated).async { [weak self] in
DispatchQueue.global(qos: .userInitiated).asyncAfter(deadline: .now() + 0.01) { [weak self] in
// If we don't have a `initialFocusedInfo` then default to `.pageBefore` (it'll query
// from a `0` offset)
switch (focusedInteractionInfo ?? initialData?.initialUnreadInteractionInfo) {
Expand Down
4 changes: 2 additions & 2 deletions Session/Home/HomeVC.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ final class HomeVC: BaseVC, LibSessionRespondingViewController, UITableViewDataS

// MARK: - UI

private var tableViewTopConstraint: NSLayoutConstraint!
private var tableViewTopConstraint: NSLayoutConstraint?

private lazy var seedReminderView: SeedReminderView = {
let result = SeedReminderView()
Expand Down Expand Up @@ -451,7 +451,7 @@ final class HomeVC: BaseVC, LibSessionRespondingViewController, UITableViewDataS

// Update the 'view seed' UI
if updatedState.showViewedSeedBanner != self.viewModel.state.showViewedSeedBanner {
tableViewTopConstraint.isActive = false
tableViewTopConstraint?.isActive = false
seedReminderView.isHidden = !updatedState.showViewedSeedBanner

if updatedState.showViewedSeedBanner {
Expand Down
11 changes: 7 additions & 4 deletions Session/Meta/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
/// There is a warning which can happen on launch because the Database read can be blocked by another database operation
/// which could result in this blocking the main thread, as a result we want to check the identity exists on a background thread
/// and then return to the main thread only when required
DispatchQueue.global(qos: .default).async { [weak self] in
DispatchQueue.global(qos: .default).asyncAfter(deadline: .now() + 0.01) { [weak self] in
guard Identity.userExists() else { return }

self?.enableBackgroundRefreshIfNecessary()
Expand Down Expand Up @@ -679,7 +679,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD

/// We want to start observing the changes for the 'HomeVC' and want to wait until we actually get data back before we
/// continue as we don't want to show a blank home screen
DispatchQueue.global(qos: .userInitiated).async {
DispatchQueue.global(qos: .userInitiated).asyncAfter(deadline: .now() + 0.01) {
viewController.startObservingChanges() {
populateHomeScreenTimer.invalidate()

Expand Down Expand Up @@ -719,7 +719,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
/// On application startup the `Storage.read` can be slightly slow while GRDB spins up it's database
/// read pools (up to a few seconds), since this read is blocking we want to dispatch it to run async to ensure
/// we don't block user interaction while it's running
DispatchQueue.global(qos: .default).async {
DispatchQueue.global(qos: .default).asyncAfter(deadline: .now() + 0.01) {
let unreadCount: Int = Storage.shared
.read { db in try Interaction.fetchUnreadCount(db) }
.defaulting(to: 0)
Expand Down Expand Up @@ -814,7 +814,10 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD

/// Start the pollers on a background thread so that any database queries they need to run don't
/// block the main thread
DispatchQueue.global(qos: .background).async { [weak self] in
///
/// **Note:** We add a delay of `0.01` to prevent potential database re-entrancy if this is triggered
/// within the completion block of a database transaction, this gives it the time to complete the transaction
DispatchQueue.global(qos: .background).asyncAfter(deadline: .now() + 0.01) { [weak self] in
self?.poller.start()

guard shouldStartGroupPollers else { return }
Expand Down
5 changes: 3 additions & 2 deletions Session/Meta/SessionApp.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,9 @@ public struct SessionApp {
)
}

/// The thread should generally exist at the time of calling this method, but on the off chance it doesn't then we need to `fetchOrCreate` it and
/// should do it on a background thread just in case something is keeping the DBWrite thread busy as in the past this could cause the app to hang
/// The thread should generally exist at the time of calling this method, but on the off chance it doesn't then we need to
/// `fetchOrCreate` it and should do it on a background thread just in case something is keeping the DBWrite thread
/// busy as in the past this could cause the app to hang
guard threadInfo?.threadExists == true else {
DispatchQueue.global(qos: .userInitiated).async {
Storage.shared.write { db in
Expand Down
2 changes: 1 addition & 1 deletion Session/Meta/Translations/InfoPlist.xcstrings

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion Session/Meta/Translations/Localizable.xcstrings
Original file line number Diff line number Diff line change
Expand Up @@ -304068,7 +304068,7 @@
"km" : {
"stringUnit" : {
"state" : "translated",
"value" : "ការបង្កើតគណនីគឺមានភ្លាមៗ, \noofសាន្ត និងមិនបង្ហាញអត្តសញ្ញាណ {emoji}"
"value" : "ការបង្កើតគណនីគឺភ្លាមៗ ឥតគិតថ្លៃ និងអនាមិក {emoji}"
}
},
"kn" : {
Expand Down
7 changes: 5 additions & 2 deletions SessionMessagingKit/Jobs/Types/GarbageCollectionJob.swift
Original file line number Diff line number Diff line change
Expand Up @@ -344,8 +344,11 @@ public enum GarbageCollectionJob: JobExecutor {
}
},
completion: { _, _ in
// Dispatch async so we can swap from the write queue to a read one (we are done writing)
queue.async {
// Dispatch async so we can swap from the write queue to a read one (we are done
// writing), this has to be done after a slight delay to ensure the transaction
// provided by the completion block completes first (ie. so we don't hit
// re-entrancy issues)
queue.asyncAfter(deadline: .now() + 0.01) {
// Retrieve a list of all valid attachmnet and avatar file paths
struct FileInfo {
let attachmentLocalRelativePaths: Set<String>
Expand Down
8 changes: 4 additions & 4 deletions SessionMessagingKit/Jobs/Types/UpdateProfilePictureJob.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ public enum UpdateProfilePictureJob: JobExecutor {
queue: queue,
displayPictureUpdate: (profilePictureData.map { .currentUserUploadImageData($0) } ?? .none),
success: { db in
// Need to call the 'success' closure asynchronously on the queue to prevent a reentrancy
// issue as it will write to the database and this closure is already called within
// another database write
queue.async {
// Need to call the 'success' closure asynchronously on the queue after a slight
// delay to prevent a reentrancy issue as it will write to the database and this
// closure is already called within another database write
queue.asyncAfter(deadline: .now() + 0.01) {
SNLog("[UpdateProfilePictureJob] Profile successfully updated")
success(job, false, dependencies)
}
Expand Down
7 changes: 4 additions & 3 deletions SessionMessagingKit/Sending & Receiving/MessageSender.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1054,9 +1054,10 @@ public final class MessageSender {

guard !rowIds.isEmpty else { return error }

// Need to dispatch to a different thread to prevent a potential db re-entrancy
// issue from occuring in some cases
DispatchQueue.global(qos: .background).async {
// Note: We need to dispatch this after a small 0.01 delay to prevent any potential
// re-entrancy issues since the 'asyncMigrate' returns a result containing a DB instance
// within a transaction
DispatchQueue.global(qos: .background).asyncAfter(deadline: .now() + 0.01, using: dependencies) {
dependencies.storage.write { db in
switch destination {
case .syncMessage:
Expand Down
7 changes: 5 additions & 2 deletions SessionSnodeKit/LibSession/LibSession+Networking.swift
Original file line number Diff line number Diff line change
Expand Up @@ -574,9 +574,12 @@ private extension LibSession {
return Log.error("[LibSession] CallbackWrapper called with null context.")
}

// Dispatch async so we don't block libSession's internals with Swift logic (which can block other requests)
/// Dispatch async so we don't block libSession's internals with Swift logic (which can block other requests), we
/// add the `0.01` delay to ensure the closure isn't executed immediately
let wrapper: CallbackWrapper<Output> = Unmanaged<CallbackWrapper<Output>>.fromOpaque(ctx).takeRetainedValue()
DispatchQueue.global(qos: .default).async { [wrapper] in wrapper.resultPublisher.send(output) }
DispatchQueue.global(qos: .default).asyncAfter(deadline: .now() + 0.01) { [wrapper] in
wrapper.resultPublisher.send(output)
}
}

public func unsafePointer() -> UnsafeMutableRawPointer { Unmanaged.passRetained(self).toOpaque() }
Expand Down
51 changes: 34 additions & 17 deletions SessionUtilitiesKit/Database/Storage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -329,9 +329,10 @@ open class Storage {
}
}()

// Note: We need to dispatch this to the next run toop to prevent any potential re-entrancy
// issues since the 'asyncMigrate' returns a result containing a DB instance
DispatchQueue.global(qos: .userInitiated).async {
// Note: We need to dispatch this after a small 0.01 delay to prevent any potential
// re-entrancy issues since the 'asyncMigrate' returns a result containing a DB instance
// within a transaction
DispatchQueue.global(qos: .userInitiated).asyncAfter(deadline: .now() + 0.01, using: dependencies) {
migrationCompleted(finalResult)
}
}
Expand Down Expand Up @@ -524,8 +525,8 @@ open class Storage {
case valid(DatabaseWriter)
case invalid(Error)

init(_ storage: Storage) {
switch (storage.isSuspended, storage.isValid, storage.dbWriter) {
init(_ storage: Storage?) {
switch (storage?.isSuspended, storage?.isValid, storage?.dbWriter) {
case (true, _, _): self = .invalid(StorageError.databaseSuspended)
case (false, true, .some(let dbWriter)): self = .valid(dbWriter)
default: self = .invalid(StorageError.databaseInvalid)
Expand Down Expand Up @@ -632,7 +633,7 @@ open class Storage {
) -> AnyPublisher<T, Error> {
switch StorageState(self) {
case .invalid(let error): return StorageState.logIfNeeded(error, isWrite: true)
case .valid(let dbWriter):
case .valid:
/// **Note:** GRDB does have a `writePublisher` method but it appears to asynchronously trigger
/// both the `output` and `complete` closures at the same time which causes a lot of unexpected
/// behaviours (this behaviour is apparently expected but still causes a number of odd behaviours in our code
Expand All @@ -642,11 +643,19 @@ open class Storage {
/// which behaves in a much more expected way than the GRDB `writePublisher` does
let info: CallInfo = CallInfo(fileName, functionName, lineNumber, true, self)
return Deferred {
Future { resolver in
do { resolver(Result.success(try dbWriter.write(Storage.perform(info: info, updates: updates)))) }
catch {
StorageState.logIfNeeded(error, isWrite: true)
resolver(Result.failure(error))
Future { [weak self] resolver in
/// The `StorageState` may have changed between the creation of the publisher and it actually
/// being executed so we need to check again
switch StorageState(self) {
case .invalid(let error): return StorageState.logIfNeeded(error, isWrite: true)
case .valid(let dbWriter):
do {
resolver(Result.success(try dbWriter.write(Storage.perform(info: info, updates: updates))))
}
catch {
StorageState.logIfNeeded(error, isWrite: true)
resolver(Result.failure(error))
}
}
}
}.eraseToAnyPublisher()
Expand Down Expand Up @@ -678,7 +687,7 @@ open class Storage {
) -> AnyPublisher<T, Error> {
switch StorageState(self) {
case .invalid(let error): return StorageState.logIfNeeded(error, isWrite: false)
case .valid(let dbWriter):
case .valid:
/// **Note:** GRDB does have a `readPublisher` method but it appears to asynchronously trigger
/// both the `output` and `complete` closures at the same time which causes a lot of unexpected
/// behaviours (this behaviour is apparently expected but still causes a number of odd behaviours in our code
Expand All @@ -688,11 +697,19 @@ open class Storage {
/// which behaves in a much more expected way than the GRDB `readPublisher` does
let info: CallInfo = CallInfo(fileName, functionName, lineNumber, false, self)
return Deferred {
Future { resolver in
do { resolver(Result.success(try dbWriter.read(Storage.perform(info: info, updates: value)))) }
catch {
StorageState.logIfNeeded(error, isWrite: false)
resolver(Result.failure(error))
Future { [weak self] resolver in
/// The `StorageState` may have changed between the creation of the publisher and it actually
/// being executed so we need to check again
switch StorageState(self) {
case .invalid(let error): return StorageState.logIfNeeded(error, isWrite: false)
case .valid(let dbWriter):
do {
resolver(Result.success(try dbWriter.read(Storage.perform(info: info, updates: value))))
}
catch {
StorageState.logIfNeeded(error, isWrite: false)
resolver(Result.failure(error))
}
}
}
}.eraseToAnyPublisher()
Expand Down
25 changes: 16 additions & 9 deletions SessionUtilitiesKit/Database/Types/PagedDatabaseObserver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -158,27 +158,34 @@ public class PagedDatabaseObserver<ObservedTable, T>: TransactionObserver where
_changesInCommit.performUpdate { $0.inserting(trackedChange) }
}

/// We will process all updates which come through this method even if 'onChange' is null because if the UI stops observing and then starts
/// again later we don't want to have missed any changes which happened while the UI wasn't subscribed (and doing a full re-query seems painful...)
/// We will process all updates which come through this method even if 'onChange' is null because if the UI stops observing and
/// then starts again later we don't want to have missed any changes which happened while the UI wasn't subscribed (and doing
/// a full re-query seems painful...)
///
/// **Note:** This function is generally called within the DBWrite thread but we don't actually need write access to process the commit, in order
/// to avoid blocking the DBWrite thread we dispatch to a serial `commitProcessingQueue` to process the incoming changes (in the past not doing
/// so was resulting in hanging when there was a lot of activity happening)
/// **Note:** This function is generally called within the DBWrite thread but we don't actually need write access to process the
/// commit, in order to avoid blocking the DBWrite thread we dispatch to a serial `commitProcessingQueue` to process the
/// incoming changes (in the past not doing so was resulting in hanging when there was a lot of activity happening)
public func databaseDidCommit(_ db: Database) {
// If there were no pending changes in the commit then do nothing
guard !self.changesInCommit.isEmpty else { return }

// Since we can't be sure the behaviours of 'databaseDidChange' and 'databaseDidCommit' won't change in
// the future we extract and clear the values in 'changesInCommit' since it's '@ThreadSafe' so will different
// threads modifying the data resulting in us missing a change
// Since we can't be sure the behaviours of 'databaseDidChange' and 'databaseDidCommit'
// won't change in the future we extract and clear the values in 'changesInCommit' since
// it's '@ThreadSafe' so will different threads modifying the data resulting in us
// missing a change
var committedChanges: Set<PagedData.TrackedChange> = []

self._changesInCommit.performUpdate { cachedChanges in
committedChanges = cachedChanges
return []
}

commitProcessingQueue.async { [weak self] in
// This looks odd but if we just use `commitProcessingQueue.async` then the code can
// get executed immediately wihch can result in a new transaction being started whilst
// we are still within the transaction wrapping `databaseDidCommit` (which we don't
// want), by adding this tiny 0.01 delay we should be giving it enough time to finish
// processing the current transaction
commitProcessingQueue.asyncAfter(deadline: .now() + 0.01) { [weak self] in
self?.processDatabaseCommit(committedChanges: committedChanges)
}
}
Expand Down
Loading