[Bug]: data race: QSLibrarian firstEntryContainingObject

[n8henrie]

Before submitting your bug report, please confirm you have completed the following steps

• Searched the issue is not already listed in the Issue List (I've searched it)
• Searched in the Quicksilver Users' Group for questions relating to tech support.
• Downloaded the latest version of Quicksilver for your system and confirmed the issue still persists.

Bug description

QS has occasional crashes that I think are related to data races, so I'd like to eliminate as many data races as possible. I'm not very familiar with the dispatch APIs, so this is a bit of a task.

If I enable thread sanitizer, open QS, and pull up a few random results, the first data race that usually comes up is in QSLibrarian firstEntryContainingObject, specifically at

Quicksilver/Quicksilver/Code-QuickStepCore/QSLibrarian.m

Line 363 in 2812fe5

return [entry.contents containsObject:object];

I've tried placing mutexes (mutices?) all over -- self, entries, entry, you name it -- the only think I've found that eliminates the data races is disabling concurrent enumeration (NSEnumerationConcurrent).

Is this a data race that we can eliminate?

/Users/n8henrie/git/qs/quicksilver/Quicksilver/Code-QuickStepCore/QSLibrarian.m:363 Data race in __42-[QSLibrarian firstEntryContainingObject:]_block_invoke at 0x121a9ec40

Location is a 40-byte heap object at 0x121a9ec40
Read of size 8 by thread 17
#0	0x0000000106cc2c68 in __42-[QSLibrarian firstEntryContainingObject:]_block_invoke at /Users/n8henrie/git/qs/quicksilver/Quicksilver/Code-QuickStepCore/QSLibrarian.m:363
#1	0x000000019330c0d0 in __NSARRAY_IS_CALLING_OUT_TO_A_BOOL_BLOCK__ ()
#2	0x0000000104ca2864 in _dispatch_client_callout2 ()
Write of size 8 by thread 1
#0	0x0000000106ca0010 in __copy_helper_block_e8_32s ()
#1	0x0000000192f1a030 in _call_copy_helpers_excp ()
#2	0x000000019330c050 in __NSArrayGetIndexesPassingTest ()
#3	0x0000000124f48ad4 in -[QSObjectActions validActionsForDirectObject:indirectObject:] at /Users/n8henrie/git/qs/quicksilver/Quicksilver/PlugIns-Main/QSCorePlugIn/Code/QSObjectActions.m:44
#4	0x0000000106cb31b8 in -[QSExecutor validActionsForDirectObject:indirectObject:fromSource:types:fileType:] at /Users/n8henrie/git/qs/quicksilver/Quicksilver/Code-QuickStepCore/QSExecutor.m:353
#5	0x0000000106cb3e00 in -[QSExecutor validActionsForDirectObject:indirectObject:] at /Users/n8henrie/git/qs/quicksilver/Quicksilver/Code-QuickStepCore/QSExecutor.m:405
#6	0x0000000106cb2948 in -[QSExecutor rankedActionsForDirectObject:indirectObject:shouldBypass:] at /Users/n8henrie/git/qs/quicksilver/Quicksilver/Code-QuickStepCore/QSExecutor.m:317
#7	0x0000000106cb250c in -[QSExecutor rankedActionsForDirectObject:indirectObject:] at /Users/n8henrie/git/qs/quicksilver/Quicksilver/Code-QuickStepCore/QSExecutor.m:298
#8	0x0000000106af7804 in -[QSInterfaceController rankedActions] at /Users/n8henrie/git/qs/quicksilver/Quicksilver/Code-QuickStepInterface/QSInterfaceController.m:326
#9	0x0000000106af7d08 in -[QSInterfaceController updateActionsNow] at /Users/n8henrie/git/qs/quicksilver/Quicksilver/Code-QuickStepInterface/QSInterfaceController.m:343
#10	0x000000019450d90c in __NSFireTimer ()
#11	0x0000000192e9c274 in start ()
Heap block allocated by thread 1
#0	0x0000000104e35fb8 in wrap_malloc ()
#1	0x00000001930403a8 in _malloc_type_malloc_outlined ()
#2	0x000000019330c050 in __NSArrayGetIndexesPassingTest ()
#3	0x0000000124f48ad4 in -[QSObjectActions validActionsForDirectObject:indirectObject:] at /Users/n8henrie/git/qs/quicksilver/Quicksilver/PlugIns-Main/QSCorePlugIn/Code/QSObjectActions.m:44
#4	0x0000000106cb31b8 in -[QSExecutor validActionsForDirectObject:indirectObject:fromSource:types:fileType:] at /Users/n8henrie/git/qs/quicksilver/Quicksilver/Code-QuickStepCore/QSExecutor.m:353
#5	0x0000000106cb3e00 in -[QSExecutor validActionsForDirectObject:indirectObject:] at /Users/n8henrie/git/qs/quicksilver/Quicksilver/Code-QuickStepCore/QSExecutor.m:405
#6	0x0000000106cb2948 in -[QSExecutor rankedActionsForDirectObject:indirectObject:shouldBypass:] at /Users/n8henrie/git/qs/quicksilver/Quicksilver/Code-QuickStepCore/QSExecutor.m:317
#7	0x0000000106cb250c in -[QSExecutor rankedActionsForDirectObject:indirectObject:] at /Users/n8henrie/git/qs/quicksilver/Quicksilver/Code-QuickStepCore/QSExecutor.m:298
#8	0x0000000106af7804 in -[QSInterfaceController rankedActions] at /Users/n8henrie/git/qs/quicksilver/Quicksilver/Code-QuickStepInterface/QSInterfaceController.m:326
#9	0x0000000106af7d08 in -[QSInterfaceController updateActionsNow] at /Users/n8henrie/git/qs/quicksilver/Quicksilver/Code-QuickStepInterface/QSInterfaceController.m:343
#10	0x000000019450d90c in __NSFireTimer ()
#11	0x0000000192e9c274 in start ()

Steps to reproduce

As above

Expected behavior

Clean tsan without data races

MacOS Version

macos15

Quicksilver Version

2.4.3

Relevant Plugins

No response

Crash Logs or Spindump

No response

Screenshots

Additional info

No response

[pjrobertson]

OK, so that method can be slightly optimised to stop once a match is found (avoiding having to keep enumerating the entries) but this indeed is a race condition. It's only caused when the 'Show source in catalog' action is enabled though - that's the only time this is used. And it's pretty wasteful if you ask me.

I added a bit of info to meta that stores an object's catalog entry info so we can check on that. Run with the latest sequoia changes to see if this improves things for you.

Again, it doesn't completely eliminate the possibility of a race, but it a) reduces the wasteful work QS was doing before so b) reduces the changes of it happening.

[n8henrie]

OK, so that method can be slightly optimised to stop once a match is found

Yes, I saw the potential opportunity for optimization, but I think that's an unrelated issue to the data race. I'm still curious about the data race -- just trying to learn here.

It's only a race if at least one thread is writing to shared memory here. A mutex around the shared memory should resolve the data race. Where is the write, and what is the shared memory that needs protection?

Tsan says the write is at:

Quicksilver/Quicksilver/PlugIns-Main/QSCorePlugIn/Code/QSObjectActions.m

Line 44 in 2812fe5

if ([[QSLibrarian sharedInstance] firstEntryContainingObject:dObject])

I don't see the write, it's just pointing back at the same method under discussion. What gives?

Thanks for the help!