Skip to content

Ensure closed connections evict from ranking #191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TimEvens merged 1 commit into from
Sep 8, 2026
Merged

Ensure closed connections evict from ranking #191

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 48 additions & 19 deletions src/client_manager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,32 +131,61 @@ namespace laps {

void ClientManager::PurgePublishState(std::uint64_t connection_handle)
{
std::lock_guard<std::mutex> _(state_.state_mutex);
// Snapshot of tracks to remove from top-n.
std::vector<std::shared_ptr<SubscribeTrackHandler>> top_n_removal;

{
std::lock_guard<std::mutex> _(state_.state_mutex);

std::vector<std::pair<std::uint64_t, std::uint64_t>> pub_subs;
for (const auto& [key, _] : state_.pub_subscribes) {
if (key.second == connection_handle) {
pub_subs.push_back(key);
// Collect publisher subscriptions for this connection.
std::vector<std::pair<std::uint64_t, std::uint64_t>> pub_subs;
for (const auto& [key, _] : state_.pub_subscribes) {
if (key.second == connection_handle) {
pub_subs.push_back(key);
}
}
}

for (const auto& remove_key : pub_subs) {
state_.pub_subscribes.erase(remove_key);
SPDLOG_LOGGER_DEBUG(LOGGER,
"Purge publish state for track_alias: {} connection handle: {}",
remove_key.first,
remove_key.second);
}
for (const auto& remove_key : pub_subs) {
auto handler = state_.pub_subscribes.at(remove_key);
state_.pub_subscribes.erase(remove_key);
SPDLOG_LOGGER_DEBUG(LOGGER,
"Purge publish state for track hash: {} connection handle: {}",
remove_key.first,
remove_key.second);

// Remove from top-n if no other connection publishes this track.
const auto next = state_.pub_subscribes.lower_bound({ remove_key.first, 0 });
if (next == state_.pub_subscribes.end() || next->first.first != remove_key.first) {
top_n_removal.emplace_back(std::move(handler));
Comment on lines +156 to +159

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Evict rankings owned by the disconnected publisher

When multiple connections publish the same track, the presence of another pub_subscribes entry does not mean the existing ranking entry is valid: TrackRanking stores only the connection ID and property value from the publisher that most recently updated that alias. If that publisher is the connection being purged, this condition skips removal and leaves its value and closed connection ID in the top-N until another publisher updates the property or the entry ages out, producing stale rankings and incorrect self-track filtering. Remove the disconnected handler's ranking entry or replace it with state from a surviving publisher rather than merely checking for the same track hash.

Useful? React with 👍 / 👎.

@RichLogan RichLogan Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably matters but I'm not sure how much attention we should pay to multi-publisher ranking right now.

}
}

std::vector<std::pair<quicr::TrackNamespace, std::uint64_t>> anno_remove_list;
for (const auto& [key, _] : state_.pub_namespace_active) {
if (key.second == connection_handle) {
anno_remove_list.push_back(key);
// Clear pub_subscribes_by_req_id for this connection.
for (auto it = state_.pub_subscribes_by_req_id.begin(); it != state_.pub_subscribes_by_req_id.end();) {
if (it->first.second != connection_handle) {
++it;
continue;
}
it = state_.pub_subscribes_by_req_id.erase(it);
}

std::vector<std::pair<quicr::TrackNamespace, std::uint64_t>> anno_remove_list;
for (const auto& [key, _] : state_.pub_namespace_active) {
if (key.second == connection_handle) {
anno_remove_list.push_back(key);
}
}

for (const auto& remove_key : anno_remove_list) {
state_.pub_namespace_active.erase(remove_key);
}
}

for (const auto& remove_key : anno_remove_list) {
state_.pub_namespace_active.erase(remove_key);
// Now remove these tracks from the top-n ranking outside the lock.
for (const auto& handler : top_n_removal) {
if (handler) {
handler->RemoveFromTrackRanking();
}
}
}

Expand Down
Loading