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

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

Open
joaomvfsantos opened this issue Nov 14, 2023 · 12 comments

Comments

@joaomvfsantos
Copy link

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
Copy link
Member

wtmoose commented Nov 14, 2023

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
Copy link
Author

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
Copy link
Member

wtmoose commented Nov 14, 2023

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
Copy link
Author

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
Copy link

IceFloe commented Nov 24, 2023

Same for me, did not see this behaviour before

@wtmoose wtmoose added the bug label Nov 30, 2023
@wtmoose
Copy link
Member

wtmoose commented Nov 30, 2023

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

@IceFloe
Copy link

IceFloe commented Nov 30, 2023

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

@wtmoose
Copy link
Member

wtmoose commented Nov 30, 2023

@IceFloe what happens?

@IceFloe
Copy link

IceFloe commented Nov 30, 2023

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 added a commit that referenced this issue Dec 3, 2023
@wtmoose
Copy link
Member

wtmoose commented Dec 3, 2023

@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
Copy link
Author

joaomvfsantos commented Dec 6, 2023

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

@wtmoose
Copy link
Member

wtmoose commented Dec 7, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants