search

quicksilver / Quicksilver

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

process monitor crashes #3010

Closed n8henrie closed 1 month ago

n8henrie commented 5 months ago
n8henrie commented 5 months ago

This is intended to fix https://github.com/quicksilver/Quicksilver/issues/3008

pjrobertson commented 2 months ago

Hey Nate 👋 :-)

Does this actually fix the problem? Have you been running with this and do you still experience crashes? If not - then it looks good to me. My only concern is: does it cause any locks anywhere else in the code?

n8henrie commented 2 months ago

Hi Pat!

Does this actually fix the problem?

No, unfortunately. I think I have mostly / partially reverted it in my local branch, will convert this to a draft to reduce confusion.

I've finally discovered xcode's thread sanitizer, which I think will be helpful in pinpointing areas of concern, but I've also been busy with a move and an upcoming board exam, so it's been slow going.

It also seem that thread sanitizer may have some false positives, as I've spent way too much time trying to hunt down the reported race condition at https://github.com/quicksilver/Quicksilver/blob/f4f679f3a9a0dc377214e150761ff633a9807a13/Quicksilver/Code-QuickStepCore/QSLibrarian.m#L362, but all I've figured out is that it goes away when I remove NSEnumerationConcurrent (and returns if I put it back in). Either that or it's somewhere in https://github.com/quicksilver/Quicksilver/blob/f4f679f3a9a0dc377214e150761ff633a9807a13/Quicksilver/Code-QuickStepCore/QSCatalogEntry.m#L734 -- I think..

pjrobertson commented 2 months ago

Hey hey!

Darn, that's a shame it doesn't work. Although I didn't think this would work - we used thread safe dicts in the past for QSProcessMonitor, but I actually took them out as they weren't helping :(

Good to hear you've got the thread sanitizer working! I wonder if ChatGPT can analyze and fix the code for us...? ;-)

n8henrie commented 2 months ago

I wonder if ChatGPT can analyze and fix the code for us...?

I've tried! Unfortunately doesn't seem to know ObjC very well either. I tried (repeatedly) to get it to recreate a toy program that would crash with EXC_BAD_ACCESS due to a race condition, but the programs it generated all ran without errors or crashing.

n8henrie commented 1 month ago

@pjrobertson actually, I take it back -- this seems to mitigate the issue at least somewhat. Without this I'm getting crashes within an hour or two, with this it's been running for 2 days without a crash. Let's merge this for now and see if it at least mitigates the issue.