[asterisk-dev] channel_walk_locked possible bug / possible fix?

[Steve Davies]

On 11/16/06, Kevin P. Fleming <kpfleming at digium.com> wrote:
> Steve Davies wrote:
> > I am attaching a patch which causes the deadlocked channel to be
> > skipped rather than returning NULL in this case. If people do not find
> > this suggestion laughably silly, I will add it to the bug tracker.
>
> I think, in fact, that this is probably a good idea. Skipping channels
> we cannot get the lock for makes much more sense than stopping the
> channel walk completely, especially when we are doing the walk to find a
> specific channel in the first place (and haven't found it yet).

Raised in Mantis with the patch attached. id=8376.

> Your point about name_prefix searching seems wrong; all the different
> variations end up calling channel_find_locked, which most certainly does
> use the name/namelen pair of arguments if they are supplied.

The reason I made the observation about name_prefix searching is
because all of the name/prefix matches are inside an
  if(!prev) {...}
block, which will never evaluate to true because the name_prefix
searching method provides a value for prev.

> I would make two observations regarding your proposed patch:
>
> 1) There are some cases where the channel_find_locked call can only
> return one channel (full-name matching, for one, there may be others).
> In those cases, returning NULL if we can't get the lock is the proper
> thing to do, because 'skipping' won't ever find that channel again.

Agreed - As far as I could tell, checking for whether prev is set
ensures that the name search behaviour is unaffected. I will check
more carefully for additional uses of channel_find_locked.

> 2) This will be a significant behavior change for the 1.2 branch, and as
> such it will require some significant testing. Once we have a proposed
> patch that everyone is happy with I can schedule some testing time in
> our BE testing lab (since this patch would end up going into BE anyway),
> but I'd like to see some others in the community that have
> stress-testing setups already built willing to help test this patch as
> well. Jeremy McNamara and Greg Boehnlein are two people that I know for
> sure have existing test networks built and test scripts that can hammer
> on Asterisk until it breaks... testing with this patch would be useful
> there.

I will of course also be testing it myself :)

Regards,
Steve