[asterisk-dev] chan->_softhangup

[Damien Wedhorn]

I am (was?) looking at changing the chan->_softhangup to be atomic so as
part of the readq locking in channel.c (reason being is that it remove a
lot - most? - of the chan locking requirements in chan_*).

Anyway, in order to work out the best way of atomicising changes to
_softhangup, I came to the conclusion that the need for having
chan->_softhangup is probably dubious at best. That's not saying that it
is not used, it actually shows up all through the code. But, it doesn't
seem to do a lot and the few things that it actually does would probably
be better dealt with in a different manner.

My digging uncovered:

You can not rely on what is in _softhangup. Sure if it does not equal
zero, something somewhere set it to something other than zero. But don't
rely on it actually meaning that the channel should be hung up. For
example, ast_pbx_run uses it for timeouts, in which case it tries the
next priority. If it is zero, something may or may not have queued a
hangup. Even ast_queue_hangup may or may not set the flag (it will
however queue a hangup frame).

If the last frame on the queue is a hangup, not other frames are added.

The bridges seem to call ast_check_hangup on every frame which basically
returns _softhangup (but also checks for timeouts) and if set destroys
the bridge. If it isn't set, it processes the frame and if the frame is
a control frame and is not of a specific sort (which a hangup frame
isn't it included) it also destroys the bridge. Given that
ast_queue_hangup may or may not set _softhangup, but reliably adds a
hangup frame, it would be better to just test if the last frame on the
queue is a hangup.

Other function that also call ast_check_hangup test if a timeout has
occurred. This is obviously redundant as I would assume once per every
frame is sufficient.

Some code explicitly sets _softhangup with looking at what is already
there effectively overwriting anything that may have been previously set.

Rather than trying to make the softhangup stuff atomic, I suggest that
we look at removing softhangup completely. Given the various and
different usage of softhangup, various things need to be undertaken.
Namely, where a test is purely to see whether a hangup is imminent, we
can test whether the last frame of the queue is a hangup. And where the
usage is to obtain feedback from some other function, the function is
either rewritten or feedback obtained another way, either through res or
an additional specific chanvar.

Given the changes will be extensive and invasive I would suggest a
staged approach where a bit is done at a time. For example the first
patch may be the pbx usage where an additional chanvar, say
chan->extentimeout or chan->pbxtimeout, is added and chan->_softhangup
is removed.

Comments, suggestions? Does anyone know of a _softhangup usage that can
not be handled in this manner?