Issues:
* The bridging core allowed multiple bridges to be created with the same
unique bridgeId at the same time. Only the last bridge created with the
duplicate name was actually saved to the core bridges container.
* The bridging core was creating a stasis topic for the bridge and saving it
in the bridge->topic field but not increasing its reference count. In the
case where two bridges were created with the same uniqueid (which is also
the topic name), the second bridge would get the _existing_ topic the first
bridge created. When the first bridge was destroyed, it would take the
topic with it so when the second bridge attempted to publish a message to
it it either FRACKed or SEGVd.
* The bridge destructor, which also destroys the bridge topic, is run from the
bridge manager thread not the caller's thread. This makes it possible for
an ARI developer to create a new one with the same uniqueid believing the
old one was destroyed when, in fact, the old one's destructor hadn't
completed. This could cause the new bridge to get the old one's topic just
before the topic was destroyed. When the new bridge attempted to publish
a message on that topic, asterisk could either FRACK or SEGV.
* The ARI bridges resource also allowed multiple bridges to be created with
the same uniqueid but it kept the duplicate bridges in its app_bridges
container. This created a situation where if you added two bridges with
the same "bridge1" uniqueid, all operations on "bridge1" were performed on
the first bridge created and the second was basically orphaned. If you
attempted to delete what you thought was the second bridge, you actually
deleted the first one created.
Changes:
* A new API `ast_bridge_topic_exists(uniqueid)` was created to determine if
a topic already exists for a bridge.
* `bridge_base_init()` in bridge.c and `ast_ari_bridges_create()` in
resource_bridges.c now call `ast_bridge_topic_exists(uniqueid)` to check
if a bridge with the requested uniqueid already exists and will fail if it
does.
* `bridge_register()` in bridges.c now checks the core bridges container to
make sure a bridge doesn't already exist with the requested uniqueid.
Although most callers of `bridge_register()` will have already called
`bridge_base_init()`, which will now fail on duplicate bridges, there
is no guarantee of this so we must check again.
* The core bridges container allocation was changed to reject duplicate
uniqueids instead of silently replacing an existing one. This is a "belt
and suspenders" check.
* A global mutex was added to bridge.c to prevent concurrent calls to
`bridge_base_init()` and `bridge_register()`.
* Even though you can no longer create multiple bridges with the same uniqueid
at the same time, it's still possible that the bridge topic might be
destroyed while a second bridge with the same uniqueid was trying to use
it. To address this, the bridging core now increments the reference count
on bridge->topic when a bridge is created and decrements it when the
bridge is destroyed.
* `bridge_create_common()` in res_stasis.c now checks the stasis app_bridges
container to make sure a bridge with the requested uniqueid doesn't already
exist. This may seem like overkill but there are so many entrypoints to
bridge creation that we need to be safe and catch issues as soon in the
process as possible.
* The stasis app_bridges container allocation was changed to reject duplicate
uniqueids instead of adding them. This is a "belt and suspenders" check.
* The `bridge show all` CLI command now shows the bridge name as well as the
bridge id.
* Response code 409 "Conflict" was added as a possible response from the ARI
bridge create resources to signal that a bridge with the requested uniqueid
already exists.
* Additional debugging was added to multiple bridging and stasis files.
Resolves: #211
pull/1092/head
George Joseph3 months agocommitted bygithub-actions[bot]
George Joseph
3 months ago
committed by
github-actions[bot]