From cbbf2891d2e5b08d24a1de3f36f1450024d1fdfd Mon Sep 17 00:00:00 2001 From: George Joseph Date: Wed, 8 May 2024 11:32:36 -0600 Subject: [PATCH] stasis_channels: Use uniqueid and name to delete old snapshots Whenver a new channel snapshot is created or when a channel is destroyed, we need to delete any existing channel snapshot from the snapshot cache. Historically, we used the channel->snapshot pointer to delete any existing snapshots but this has two issues. First, if something (possibly ast_channel_internal_swap_snapshots) sets channel->snapshot to NULL while there's still a snapshot in the cache, we wouldn't be able to delete it and it would be orphaned when the channel is destroyed. Since we use the cache to list channels from the CLI, AMI and ARI, it would appear as though the channel was still there when it wasn't. Second, since there are actually two caches, one indexed by the channel's uniqueid, and another indexed by the channel's name, deleting from the caches by pointer requires a sequential search of all of the hash table buckets in BOTH caches to find the matching snapshots. Not very efficient. So, we now delete from the caches using the channel's uniqueid and name. This solves both issues. This doesn't address how channel->snapshot might have been set to NULL in the first place because although we have concrete evidence that it's happening, we haven't been able to reproduce it. Resolves: #783 --- main/stasis_channels.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/main/stasis_channels.c b/main/stasis_channels.c index 1b5fcbf70b..a84f18968e 100644 --- a/main/stasis_channels.c +++ b/main/stasis_channels.c @@ -942,8 +942,8 @@ void ast_channel_publish_final_snapshot(struct ast_channel *chan) return; } - ao2_unlink(channel_cache, update->old_snapshot); - ao2_unlink(channel_cache_by_name, update->old_snapshot); + ao2_find(channel_cache, ast_channel_uniqueid(chan), OBJ_SEARCH_KEY | OBJ_UNLINK | OBJ_NODATA); + ao2_find(channel_cache_by_name, ast_channel_name(chan), OBJ_SEARCH_KEY | OBJ_UNLINK | OBJ_NODATA); ast_channel_snapshot_set(chan, NULL); @@ -1096,17 +1096,15 @@ void ast_channel_publish_snapshot(struct ast_channel *chan) * snapshot is not in the cache. */ ao2_wrlock(channel_cache); - if (update->old_snapshot) { - ao2_unlink_flags(channel_cache, update->old_snapshot, OBJ_NOLOCK); - } + ao2_find(channel_cache, ast_channel_uniqueid(chan), OBJ_SEARCH_KEY | OBJ_UNLINK | OBJ_NODATA | OBJ_NOLOCK); + ao2_link_flags(channel_cache, update->new_snapshot, OBJ_NOLOCK); ao2_unlock(channel_cache); /* The same applies here. */ ao2_wrlock(channel_cache_by_name); - if (update->old_snapshot) { - ao2_unlink_flags(channel_cache_by_name, update->old_snapshot, OBJ_NOLOCK); - } + ao2_find(channel_cache_by_name, ast_channel_name(chan), OBJ_SEARCH_KEY | OBJ_UNLINK | OBJ_NODATA | OBJ_NOLOCK); + ao2_link_flags(channel_cache_by_name, update->new_snapshot, OBJ_NOLOCK); ao2_unlock(channel_cache_by_name);