afpd: improved Spotlight RPC logic and bugfixes#2900
Merged
Conversation
🤖 Augment PR SummarySummary: This PR improves AFP Spotlight RPC compatibility with macOS, aligning responses and state handling more closely to observed client behavior. Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
174da71 to
f466255
Compare
Member
Author
|
@andylemin any idea why we got such a funky looking flamegraph? |
andylemin
approved these changes
Apr 19, 2026
f466255 to
04ddadb
Compare
The struct stat sb was declared but never populated, causing add_filemeta() to read garbage data for file size, owner UIDs/GIDs, and modification time in every Spotlight query result. Add a stat() call before access(), and skip the result on failure rather than propagating stale metadata.
…ts()
Previously only SLQ_STATE_RUNNING returned status 35 ("results pending");
SLQ_STATE_RESULTS and SLQ_STATE_FULL fell through to status 0, which tells
the client the query is finished even while the result buffer is still
filling. This could cause the client to stop polling prematurely.
The correct condition, is state < SLQ_STATE_DONE: all of RUNNING, RESULTS, and
FULL return 0x23 (35); DONE and beyond return 0.
Samba's mdssvc maps this selector to the same handler as fetchAttributes:forOIDArray:context:. Add it to the dispatch chain to avoid logging an "unknown Spotlight RPC" error when macOS clients send this variant.
…lemeta() kMDItemContentModificationDate is an alias for kMDItemFSContentChangeDate, both mapping to st_mtim. Samba's mdssvc handles both spellings; without this, clients requesting the former receive nil instead of the mtime.
Add birth time support for the kMDItemContentCreationDate Spotlight attribute, using st_birthtimespec where available (macOS, FreeBSD). On platforms without birth time in struct stat the attribute returns nil, matching the prior behaviour for unknown attributes. Adds a meson.build detection for HAVE_STRUCT_STAT_ST_BIRTHTIMESPEC.
Add keys observed in macOS Spotlight server responses (via Samba traces): - kMDSStoreMetaScopes: add kMDQueryScopeAllIndexed and kMDQueryScopeComputerIndexed alongside the existing kMDQueryScopeComputer - kMDSVolumeUUID: echo the volume UUID under the second key name - kMDSStoreIsBackup: reflect actual volume configuration (true when "time machine = yes", false otherwise) rather than hardcoding a value - kMDSStoreSupportsVolFS: set true, as observed from macOS
macOS expects filenames and paths in NFD (decomposed) Unicode form. Server-side paths from Tracker are NFC (composed). Without conversion, Spotlight results with non-ASCII characters in their names may not match what the macOS client expects. Convert the path to NFD before returning kMDItemDisplayName, kMDItemFSName, _kMDItemFileName and kMDItemPath using the existing charset_decompose(CH_UTF8) facility. Falls back to the original path if the conversion buffer cannot be allocated or conversion fails.
- tracker_cursor_cb: log reason for each skipped result (stat failed, access denied, no CNID, CNID not in client filter) and log path+CNID for each accepted result; downgrade missing CNID from log_error to log_debug as it is expected when files are moved or deleted after Tracker indexed them - add_results: log result count and status code when dispatching a batch to the client - sl_rpc_fetchQueryResultsForContext: log ctx1/ctx2 at entry and emit an error log when no matching query is found for the context
add_results() transfers ownership of query_results to the client array and sets slq->query_results = NULL before calling create_result_handle(). If create_result_handle() fails the state becomes SLQ_STATE_ERROR but query_results stays NULL; a subsequent async cursor callback would then dereference it in the initial LOG and tracker_to_unix_path(), crashing under low-memory or other error paths. Add an early NULL check at the top of tracker_cursor_cb() so the callback returns cleanly in this case.
in this codebase we use the no final semicolon pattern for macros elsewhere, so let's do the same in dalloc to avoid empty statements in usage sites
04ddadb to
387c0cb
Compare
|
Contributor
🔥 Spectest (AFP 3.4) - Flamegraph (AFP_ASSERT active)Commit: 🔥 Open interactive Flamegraph (SVG) 🔝 Top 10 leaf functions
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.




a range of improvements and bugfixes that should align closer to macOS Spotlight RPC behavior, based on analysis of Samba RPC logic