[asterisk-dev] sig_ss7 softhangup

[Kaloyan Kovachev]

On Wed, 07 Mar 2012 11:29:50 -0600 (CST), Richard Mudgett
<rmudgett at digium.com> wrote:
> 
> New features and architecture enhancements are done to trunk.  That is
> its purpose.  Patches are not guaranteed to be applicable between
revisions
> of a release series much less between branches.  Trunk just went through
an
> ast_channel opaquification phase to remove layout dependencies to the
> ast_channel struct throughout the codebase.
> 

Yes, that's true. I will make separate versions of the patch for trunk, 10
and will also try to make a version for 1.8 too and provide a link in the
issue.

>> 
>> Is there a reason not to use ast_softhangup_nolock() in all places,
>> which
>> in addition queues a frame to the channel and interrupts any
>> blockers?
> 
> The use of ast_softhangup_nolock() requires channel locking
> considerations because ast_softhangup_nolock() calls ast_queue_frame()
> internally which locks the channel.  The nolock suffix is intended to
> imply that you already have the channel lock held before calling.
> 
> The _softhangup channel value is not supposed to be set directly because
> ast_softhangup() needs to potentially wake up the media handling thread
> early.  The reason _softhangup is still set directly is because there
> were so many channel drivers that did so before ast_softhangup() was
> created.  The ast_softhangup() call also has different assumptions that
> need to be considered before converting code that directly sets the
> _softhangup flags to calling ast_softhangup/ast_softhangup_nolock.
> 
> The new places in sig_ss7_indicate() that are setting the _softhangup
> flags should be changed to ast_softhangup_nolock() since the channel
> lock is already held.  I was using the code in sig_pri.c as a guide
> to how to handle the control frames.
> 
> The only other place where the _softhangup flags are set directly in
> sig_ss7.c is ss7_hangup_cics().  To convert it to using the
> ast_softhangup_nolock() requires locking the owner with
> sig_ss7_lock_owner() to acquire the channel lock without deadlock.
> 
> I will make both of these changes.
> 

Thank you, for the detailed explanation and the changes. I will check the
usage of ast_softhangup_nolock() in the other places of the patch, before
posting the next version on reviweboard.