applivery/applivery-ios-sdk

Running UI Kit stuff in background thread

SPopenko opened this issue · 5 comments

SPopenko commented

Guys, this code is running in the background thread. This is wrong, UI code should be executed on main thread only.

	fileprivate func rootIsReady() -> Bool {
		let app = UIApplication.shared
		let window = **app.keyWindow**
		guard let rootVC = window?.rootViewController else { return false }

		return rootVC.isViewLoaded
	}
Alesete commented

Hello @SPopenko, thanks for your feedback!

We were aware of that (xcode has been warning us like crazy!), and it was just fixed yesterday in develop (commit 6fcdd5b)

This fix will be included in next release v2.6 (probably today)

SPopenko commented

@Alesete great! thank you!

SPopenko commented

@Alesete I've reviewed that changes and unfortunately we need this ticket to be reopened.
No we have 2 issues with this pice of code:

	private func rootIsReady() -> Bool {
		let app = UIApplication.shared
		var window: UIWindow?
		var rootVC: UIViewController?
		runOnMainThread { <-------------------------This couse data race condition. runOnMainThread is async it means you could simultaniusly access rootVC from 2 threads.  
			window = app.keyWindow
			rootVC = window?.rootViewController
		}
		Thread.sleep(forTimeInterval: 0.1)
		guard let unwrappedRootVC = rootVC else { return false }
		return unwrappedRootVC.isViewLoaded <---------------------- This is usage of UIKit in background. 
	}

Here is offered solution:

	private func rootIsReady() -> Bool {
        var isReady = false
        // Wait until main thread complete check
		DispatchQueue.main.sync {
            isReady = UIApplication
                .shared.keyWindow?
                .rootViewController?
                .isViewLoaded ?? false
		}
        return isReady
	}

It's workable and follows the existing design pattern but I would recommend using Dispatch groups for strategy in general. But that is another story.
Also enabling thread sanitizer could help a lot!

  • 👍2
CesarTrigo commented

Hi @SPopenko , thank you for the contribution.
I have reopened the issue to “Under investigation” and we’ll come back to you ASAP.
Best,
C.

Alesete commented

Hi again @SPopenko !

You are totally right, I was afraid of blocking the main thread and freeze the app, but I test your solution and works pretty well!

Next time, if you want, you can make a Pull Request so you can have the attribution of your piece of code. But, at least, I will mention you in the release notes 😉

Thank you very much for warning us and for the solution!

  • ❤️1