[Asterisk-code-review] ARI: Detect duplicate channel IDs (asterisk[13])

[Mark Michelson]

Mark Michelson has posted comments on this change. ( https://gerrit.asterisk.org/4150 )

Change subject: ARI: Detect duplicate channel IDs
......................................................................


Patch Set 1:

(1 comment)

https://gerrit.asterisk.org/#/c/4150/1/main/channel_internal_api.c
File main/channel_internal_api.c:

Line 1508: 	if (assignedids && (does_id_conflict(assignedids->uniqueid) || does_id_conflict(assignedids->uniqueid2))) {
> This still allows a window where two threads could create the same channel 
Hard question to answer. If you have a faulty ARI program that is using the same ID for channels, and you're originating quickly enough, you'd probably end up hitting the race and end up with channels with duplicate IDs. So that's not good.

What are the alternatives?

1) Create a lock around channel creation. This would ensure that no thread could create a channel until another thread had a result from their attempt. This way, you could ensure that the inserted code here would always catch the duplicate. The downside is that this creates more contention for channel creation than previously existed, leading to a theoretical slowdown.

2) Create a second channel container hashed on uniqueid. Use AO2 options that cause the container to reject duplicates. This way, as part of channel creation, you'll detect the duplicate failure as a part of trying to link the object into the uniqueid container. The upside to this is that we would now have a much quicker way to find channels by uniqueid than we currently do. The possible downside to this is that I have no earthly clue how this might affect masquerades.

I'm not really sure if the alternatives are better or not. Thoughts?


-- 
To view, visit https://gerrit.asterisk.org/4150
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibba7ae68842dab6df0c2e9c45559208bc89d3d06
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-HasComments: Yes