diff options
| author | pepper <3782201+rohvani@users.noreply.github.com> | 2026-09-21 17:16:55 -0400 |
|---|---|---|
| committer | pepper <3782201+rohvani@users.noreply.github.com> | 2026-09-23 15:24:02 -0400 |
| commit | 85932e1ae52b02ff59b3677e5ba3ceec8114b01b (patch) | |
| tree | 5c62d0333a0500b39717c4c362ad60b6226c52c9 | |
| parent | 0803c304ced468abb761b48f6fecdda6e529ac41 (diff) | |
Remove redundant Chat Service history bookkeeping
Use strict descending TimeUUID validation to exclude duplicate page IDs,
and deterministic conversation identity to exclude duplicate direct
conversations. Add regression assertions for duplicate rows and cursor
replay while removing the extra ID sets and unused summary count.
Share regular-file inspection between scanning and mutation. Clear the
manifest dirty latch before its worker wait so callbacks can request a
follow-up, instead of comparing complete snapshots afterward.
| -rw-r--r-- | indra/newview/llchatservicehistory.cpp | 69 | ||||
| -rw-r--r-- | indra/newview/llchatservicehistorycore.cpp | 52 | ||||
| -rw-r--r-- | indra/newview/llchatservicehistorycore.h | 3 | ||||
| -rw-r--r-- | indra/newview/tests/llchatservicehistorycore_test.cpp | 8 |
4 files changed, 41 insertions, 91 deletions
diff --git a/indra/newview/llchatservicehistory.cpp b/indra/newview/llchatservicehistory.cpp index c21b5e2935..23232e794e 100644 --- a/indra/newview/llchatservicehistory.cpp +++ b/indra/newview/llchatservicehistory.cpp @@ -134,7 +134,6 @@ struct Summary { // Logical validity and retained-row count. ESummary state = SUMMARY_UNPREPARED; - U32 rows = 0; bool has_rows = false; // Physical identity captured by the last successful scan or publication. @@ -428,31 +427,6 @@ bool parseDecimalTicks(const std::string& text, U64& value) return true; } -bool inspectRegular(const std::string& path, bool& exists) -{ - // Owned artifacts must be regular files; directories and reparse/symlink paths - // fail inspection rather than being followed. - exists = false; -#if LL_WINDOWS - const std::wstring wide = ll_convert<std::wstring>(path); - const DWORD attributes = GetFileAttributesW(wide.c_str()); - if (attributes == INVALID_FILE_ATTRIBUTES) - { - return GetLastError() == ERROR_FILE_NOT_FOUND || GetLastError() == ERROR_PATH_NOT_FOUND; - } - exists = true; - return !(attributes & (FILE_ATTRIBUTE_DIRECTORY | FILE_ATTRIBUTE_REPARSE_POINT)); -#else - struct stat status; - if (::lstat(path.c_str(), &status) != 0) - { - return errno == ENOENT; - } - exists = true; - return S_ISREG(status.st_mode); -#endif -} - bool syncDirectory(const std::string& path) { #if LL_WINDOWS @@ -829,7 +803,6 @@ Summary summaryFromScan(const ArchiveScan& scan) { Summary result; result.state = scan.state == ARCHIVE_ABSENT ? SUMMARY_ABSENT : SUMMARY_VALID; - result.rows = scan.row_count; result.has_rows = scan.has_oldest; result.file_exists = scan.state != ARCHIVE_ABSENT; result.file_size = scan.file_size; @@ -994,7 +967,6 @@ bool publishRows(const std::string& path, const std::vector<Row>& rows, // Advance the in-memory summary only after the filesystem mutation succeeds. resulting.state = SUMMARY_VALID; - resulting.rows += static_cast<U32>(rows.size()); if (!resulting.has_rows) { resulting.oldest = rows.front().key; @@ -1549,7 +1521,6 @@ void syncResident(const LLUUID& id, const CapabilityContext& context, U32 epoch) // Stage a complete bounded pass in memory. No canonical bytes change until every // traversed page validates and reaches a terminal condition or durable seam. std::vector<Row> staged; - std::set<std::string> pass_ids; size_t staged_bytes = 0; U32 returned_rows = 0; std::string cursor; @@ -1754,14 +1725,6 @@ void syncResident(const LLUUID& id, const CapabilityContext& context, U32 epoch) bool reached_stored_newest = false; for (const Row& row : page.rows) { - if (!pass_ids.insert(row.msg_id).second) - { - staged.clear(); - complete = false; - failResidentPass(id, page_resident); - sRuntime.active_resident.setNull(); - return; - } ++returned_rows; if (returned_rows > MAX_PASS_ROWS) { @@ -2319,8 +2282,9 @@ void fillMetadataSlots() bool updateIndex(U32 epoch) { - // Capture one stable manifest input, regenerate it under the storage mutex, then - // detect mutations that arrived while the General-queue job was suspended. + // Clear the dirty latch before suspension so metadata callbacks can request + // another pass while this captured manifest is being written. + sRuntime.index_dirty = false; std::vector<LLUUID> archives; std::map<LLUUID, LLAvatarName> names; for (const auto& pair : sRuntime.residents) @@ -2346,31 +2310,8 @@ bool updateIndex(U32 epoch) { return false; } - if (updated) - { - std::vector<LLUUID> current_archives; - std::vector<LLUUID> current_names; - for (const auto& pair : sRuntime.residents) - { - if (pair.second.summary.state == SUMMARY_VALID && - pair.second.summary.file_exists) - { - current_archives.push_back(pair.first); - } - if (pair.second.metadata.state == META_RESOLVED) - { - current_names.push_back(pair.first); - } - } - std::vector<LLUUID> indexed_names; - for (const auto& pair : names) - { - indexed_names.push_back(pair.first); - } - sRuntime.index_dirty = archives != current_archives || indexed_names != current_names; - return true; - } - return false; + sRuntime.index_dirty |= !updated; + return updated; } void expireMetadata() diff --git a/indra/newview/llchatservicehistorycore.cpp b/indra/newview/llchatservicehistorycore.cpp index fc66b1e165..20e27597f7 100644 --- a/indra/newview/llchatservicehistorycore.cpp +++ b/indra/newview/llchatservicehistorycore.cpp @@ -190,29 +190,29 @@ namespace value = field.asString(); return true; } +} - bool inspectRegularFile(const std::string& path, bool& exists) - { - exists = false; +bool inspectRegular(const std::string& path, bool& exists) +{ + exists = false; #if LL_WINDOWS - const DWORD attributes = GetFileAttributesW(ll_convert<std::wstring>(path).c_str()); - if (attributes == INVALID_FILE_ATTRIBUTES) - { - return GetLastError() == ERROR_FILE_NOT_FOUND || - GetLastError() == ERROR_PATH_NOT_FOUND; - } - exists = true; - return !(attributes & (FILE_ATTRIBUTE_DIRECTORY | FILE_ATTRIBUTE_REPARSE_POINT)); + const DWORD attributes = GetFileAttributesW(ll_convert<std::wstring>(path).c_str()); + if (attributes == INVALID_FILE_ATTRIBUTES) + { + return GetLastError() == ERROR_FILE_NOT_FOUND || + GetLastError() == ERROR_PATH_NOT_FOUND; + } + exists = true; + return !(attributes & (FILE_ATTRIBUTE_DIRECTORY | FILE_ATTRIBUTE_REPARSE_POINT)); #else - struct stat status; - if (::lstat(path.c_str(), &status) != 0) - { - return errno == ENOENT; - } - exists = true; - return S_ISREG(status.st_mode); -#endif + struct stat status; + if (::lstat(path.c_str(), &status) != 0) + { + return errno == ENOENT; } + exists = true; + return S_ISREG(status.st_mode); +#endif } bool TimeUuidKey::operator==(const TimeUuidKey& rhs) const @@ -781,7 +781,6 @@ bool validateConversationList(const LLSD& value, const LLUUID& agent_id, } std::set<LLUUID> residents; - std::set<std::string> conversations; for (LLSD::array_const_iterator it = value.beginArray(); it != value.endArray(); ++it) { // Every list row must have a known type and bounded canonical identifiers, @@ -812,8 +811,7 @@ bool validateConversationList(const LLSD& value, const LLUUID& agent_id, resident == agent_id || !parseTimeUuid(token, token_key) || conversation != directConversationId(agent_id, resident) || - !residents.insert(resident).second || - !conversations.insert(conversation).second) + !residents.insert(resident).second) { return false; } @@ -863,12 +861,12 @@ bool validateHistoryPage(const LLSD& value, const LLUUID& agent_id, return false; } - std::set<std::string> ids; TimeUuidKey previous; bool have_previous = false; // Validate every row before the page can affect scheduler or archive state. - // UUIDv1 keys must be unique, strictly descending, and below the request cursor. + // Strictly descending UUIDv1 keys below the cursor exclude duplicate IDs + // both within this page and across earlier pages in the same pass. for (LLSD::array_const_iterator it = value["messages"].beginArray(); it != value["messages"].endArray(); ++it) { @@ -882,7 +880,7 @@ bool validateHistoryPage(const LLSD& value, const LLUUID& agent_id, !(*it)["dialog"].isInteger() || !llsdString(*it, "created_at", row.created_at) || row.conversation_id != conversation_id || - !parseTimeUuid(row.msg_id, row.key) || !ids.insert(row.msg_id).second || + !parseTimeUuid(row.msg_id, row.key) || !parseCanonicalUuid(from_text, row.from_id) || (row.from_id != agent_id && row.from_id != resident_id) || !cleanText(row.from_name, 256) || !cleanText(row.message, 1024) || @@ -950,7 +948,7 @@ void writeCsvRow(std::ostream& output, const Row& row) bool archiveStamp(const std::string& path, U64& file_size, S64& file_mtime) { bool exists = false; - if (!inspectRegularFile(path, exists) || !exists) + if (!inspectRegular(path, exists) || !exists) { return false; } @@ -983,7 +981,7 @@ bool scanArchive(const std::string& path, const LLUUID& agent_id, // Only a regular canonical path may participate in archive reads. bool exists = false; - if (!inspectRegularFile(path, exists)) + if (!inspectRegular(path, exists)) { scan.state = ARCHIVE_FAILED; return false; diff --git a/indra/newview/llchatservicehistorycore.h b/indra/newview/llchatservicehistorycore.h index ebf47e809e..b586b45f94 100644 --- a/indra/newview/llchatservicehistorycore.h +++ b/indra/newview/llchatservicehistorycore.h @@ -174,6 +174,9 @@ namespace LLChatServiceHistoryCore // Capture the regular file's physical identity for guarded repair and publication. bool archiveStamp(const std::string& path, U64& file_size, S64& file_mtime); + + // Owned artifacts may be absent or regular files, but never symlinks or directories. + bool inspectRegular(const std::string& path, bool& exists); } #endif diff --git a/indra/newview/tests/llchatservicehistorycore_test.cpp b/indra/newview/tests/llchatservicehistorycore_test.cpp index d3d3548728..8c67d32897 100644 --- a/indra/newview/tests/llchatservicehistorycore_test.cpp +++ b/indra/newview/tests/llchatservicehistorycore_test.cpp @@ -136,6 +136,14 @@ template<> template<> void object_t::test<5>() wirePage(rows), AGENT, RESIDENT, directConversationId(AGENT, RESIDENT), "", 0, page)); + // Ordering alone rejects duplicates, including replay of the preceding page's cursor. + rows[1] = rows[0]; + ensure("duplicate row rejected", !validateHistoryPage( + wirePage(rows), AGENT, RESIDENT, directConversationId(AGENT, RESIDENT), "", 0, page)); + rows.erase(1); + ensure("cursor row cannot repeat on the next page", !validateHistoryPage( + wirePage(rows, FIRST), AGENT, RESIDENT, directConversationId(AGENT, RESIDENT), FIRST, 0, page)); + LLSD wrong = wirePage(LLSD::emptyArray()); wrong["limit"] = "100"; ensure("coercible type rejected", !validateHistoryPage( |
