quicksilver / Quicksilver

Quicksilver Project Source
http://qsapp.com
Apache License 2.0
2.73k stars 285 forks source link

Crash updating UI on background thread #2668

Closed pjrobertson closed 2 years ago

pjrobertson commented 2 years ago

Describe the bug Xcode gives a Unexpected outstanding background CATransaction message and a crash when opening the UI while also checking for updates.

To Reproduce Steps to reproduce the behavior:

  1. Turn on 'Check for Updates' → 'On Launch'
  2. Restart Quicksilver
  3. Immediately open up the preferences, and then scroll on any scroll view
  4. See Quicksilver Crash

Screenshots

https://user-images.githubusercontent.com/150431/157686741-264db3cc-ff5c-4331-b4b8-37d852ba9b54.mov

What version of macOS? 10.14.6

What version of Quicksilver? Master + 1.6.1 I thought it was because of my recent changes to add a spinning wheel in the UI to show when updating, but I went back to the current release and I still got the same problem.

pjrobertson commented 2 years ago

I'm looking at this and thinking it's a macOS issue, so it'd be great if either @skurfer or @n8henrie could try and reproduce.

Another way to reproduce is to immediately click the 'check now' button to check for updates, then scroll the left sidebar:

https://user-images.githubusercontent.com/150431/157692649-55f8b9b5-65c6-4453-b035-17851980e41a.mov

skurfer commented 2 years ago

I remember consistently seeing a crash when checking for updates because it was trying to show the alert window on a background thread. I thought we fixed it, but can’t find the PR. Maybe it was fixed in an OS update?

In any case, I can’t reproduce the crash using either of the methods described here.

pjrobertson commented 2 years ago

Reopening as I found out the reason (plugin updates were running on a background thread and then causing issues when updating the interface).

Fixed by #2671

pjrobertson commented 2 years ago

Fixed by #2671

pjrobertson commented 2 years ago

Thanks for the debugging.

OK, so I think this is a macOS bug then. I debugged, and the NSAlert is getting called on the main thread, but what’s happening is this:

  1. NSAlert gets called on main thread, blocking it
  2. The OS decides to use NSConcurrentScreenUpdate to update the UI (I can’t find a way not to do this)
  3. The OS tried to do an update on the background thread
  4. Boom

I’ve only noticed this bug now because I turned on ‘Check for updates on launch’ to debug some other stuff. I think this is a pretty rare edge case, so I won’t spend any more time on it.

On 12 Mar 2022, at 08:59, Rob McBroom @.***> wrote:

I remember consistently seeing a crash when checking for updates because it was trying to show the alert window on a background thread. I thought we fixed it, but can’t find the PR. Maybe it was fixed in an OS update?

In any case, I can’t reproduce the crash using either of the methods described here.

— Reply to this email directly, view it on GitHub https://github.com/quicksilver/Quicksilver/issues/2668#issuecomment-1065765229, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABEXH6NMLCKBMY26I5SFYLU7PUAHANCNFSM5QM3PH7Q. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you authored the thread.