quicksilver / Quicksilver

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

Hangs related to QSTask #2661

Closed skurfer closed 2 years ago

skurfer commented 2 years ago

I’ve been seeing QS “Not Responding” with the latest Universal builds from Xcode 13.

I’ve sampled the process and gotten some spindumps, but in every case, it appears to be related to the @synchronized calls in QSTask.

This happened the first 3 times I tried to install plug-in updates, but not the 4th. I’ve also seen it happen when rescanning the QSProcessObjectSource, but that thing gets rescanned every time you switch applications in macOS, so it’s probably a potential issue with all catalog entries and that one is just the most likely to hit it.

I’ve attached a sample. I plan to look into this when I can, but if anyone else is bored, feel free.

Sample of Quicksilver.txt

pjrobertson commented 2 years ago

Do you have your TaskViewer pop up automatically by any chance, or is the task viewer visible somewhere (perhaps off-screen)?

Again, this looks like a QSTask issue as opposed to an actual catalog scanning issue, so that's probably where I'd look first.

I'm thinking that the task is trying to start (on the main thread) which then means a UI refresh is needed, but that can't be done because the task is blocking the main thread. Or something like that at last.

skurfer commented 2 years ago

This seems to always be caused by -[QSLibrarian reloadSource:] and -[QSLibrarian scanCatalogIgnoringIndexes:] getting called on different threads.

I think the answer might be to wrap reloadSource: with

dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_LOW, 0), ^{})

So they’ll both be run in the same queue and can’t lock each other. Sound right?

pjrobertson commented 2 years ago

Yes, that was my immediate thought as well, although I suggest you use the built-in QSGCDAsync, OR we create a specific queue for rescan tasks and use that queue (I have a vague recollection that that's been done/I did that before - but not sure)

[goes and looks] - yep, here's my PR: https://github.com/quicksilver/Quicksilver/pull/1497

Doesn't look like there was any real reason why that wasn't merged. And you said you tried it out and it seemed OK. Perhaps it's worth re-implementing those comments and seeing. But yeah, I would recommend creating a single named thread for catalog scanning, using dispatch_queue_create and storing it as an instance variable of QSLibrarian. (see here and here )

pjrobertson commented 2 years ago

Note: the fix for this (to move scanning to background threads and make it async) is good - doesn't lock the UI. However it now causes the issue at #2718

Currently debugging