SwiftKickMobile / SwiftMessages

A very flexible message bar for UIKit and SwiftUI.
MIT License
7.24k stars 741 forks source link

window being accessed from background thread when dequeueNext is called #535

Open joaomvfsantos opened 7 months ago

joaomvfsantos commented 7 months ago

dequeueNext is being called from a background queue here.

This in turn does a check to the isOrphaned property here.

The isOrphaned property does a check to the view.window property here.

This triggers an XCode warning with "UIView.window must be used from main thread only". This can lead to possible crashes.

wtmoose commented 7 months ago

Don't enqueue view on a background thread. You an either move onto the main queue yourself or call the version of .show() that takes a view provider and the library will move onto the main queue before invoking your view provider.

joaomvfsantos commented 7 months ago

Hello @wtmoose ty for the quick reply.

Maybe I did not understood your comment, but I am not enqueueing views in a background thread. Please take a look at the following view controller sample (i can also provide the full xcode project if you prefer):

class ViewController: UIViewController {

    override func viewDidLoad() {
        super.viewDidLoad()

        self.view.backgroundColor = .gray
    }

    private func showToastWithMessage(_ message: String) {
        SwiftMessages.hideAll()

        let toastView = ToastView()
        toastView.titleLabel.text = message

        var config = SwiftMessages.defaultConfig
        config.ignoreDuplicates = false
        config.duration = .seconds(seconds: 3)
        config.presentationContext = .window(windowLevel: .normal)
        SwiftMessages.show(config: config, view: toastView)
    }

    override func pressesBegan(_ presses: Set<UIPress>, with event: UIPressesEvent?) {
        if let key = presses.first?.key {
            self.showToastWithMessage("Press begand with key code: \(key.keyCode.rawValue)")
        }
        super.pressesBegan(presses, with: event)
    }

    override func pressesEnded(_ presses: Set<UIPress>, with event: UIPressesEvent?) {
        if let key = presses.first?.key {
            self.showToastWithMessage("Press ended with key code: \(key.keyCode.rawValue)")
        }
        super.pressesEnded(presses, with: event)
    }

}

You can try this out by simply runing it on a simulator, pressing the Mac keyboard keys in quick succession. This will fire the pressesBegan and pressesEnded methods (are fired by UIKit in the main thread), which will trigger the warning on the console and on the issue navigator in XCode. ToastView is just an UIView with a label.

I hope this clarifies the issue.

wtmoose commented 7 months ago

Sorry about the misunderstanding. I'm able to reproduce accessing isOrphaned on a background queue. However, I don't see any warnings. Where are you seeing that?

joaomvfsantos commented 7 months ago

If you start pressing keys on your keyboard in quick successing eventually you will get a warning log message in the console and one in the Xcode issues tab, like so:

Screenshot 2023-11-15 at 09 43 07
IceFloe commented 7 months ago

Same for me, did not see this behaviour before

wtmoose commented 7 months ago

Doesn't seem like it is breaking any functionality, but on my todo list.

IceFloe commented 7 months ago

Actually it is for me, when app is going from background to foreground, but I just using version prior to this change now.

wtmoose commented 7 months ago

@IceFloe what happens?

IceFloe commented 7 months ago

I am doing showing and hiding full screen view based on library presentation logic from the MainActor and it is lagging a little because of this error and not hiding, with old version everything is ok.

func showProgress() {
        let view = viewForHUD()
        let progress = ActivityIndicatorView()
        var config = SwiftMessages.defaultConfig
        config.duration = .indefinite(delay: 0.4, minimum: 0)
        config.presentationContext = .view(view)
        config.presentationStyle = .center
        config.dimMode = .gray(interactive: false)

        SwiftMessages.show(config: config, view: progress)
    }

    func hideProgress() {
        SwiftMessages.hide(id: ActivityIndicatorView.id)
    }
wtmoose commented 7 months ago

@joaomvfsantos Could you pull down the head of master and confirm that the issue is resolved for you?

While fixing this, I made quite a few changes:

  1. Bumped the minimum deployment target to 13.0
  2. Made SwiftMessages, Presenter and some other types @MainActor
  3. Removed the needless complexity of background execution from SwiftMessages
  4. Replaced dispatch queues with async/await
joaomvfsantos commented 7 months ago

@wtmoose I can no longer reproduce this when using the master branch, so it seems to be fixed. Thank you!

wtmoose commented 7 months ago

Great. I'm going to release this as a beta for a while before making it an official release due to the number of changes.