[asterisk-bugs] [Asterisk 0012903]: REGRESSION: chan_iax2.c patch 122259 results in chan_iax2.c sending no PING or LAGRQs

[noreply at bugs.digium.com]

A NOTE has been added to this issue. 
====================================================================== 
http://bugs.digium.com/view.php?id=12903 
====================================================================== 
Reported By:                stevedavies
Assigned To:                Corydon76
====================================================================== 
Project:                    Asterisk
Issue ID:                   12903
Category:                   Channels/chan_iax2
Reproducibility:            always
Severity:                   major
Priority:                   normal
Status:                     ready for testing
Asterisk Version:           SVN 
SVN Branch (only for SVN checkouts, not tarball releases):  1.4  
SVN Revision (number only!): 124182 
Disclaimer on File?:        N/A 
Request Review:              
====================================================================== 
Date Submitted:             06-20-2008 08:05 CDT
Last Modified:              06-27-2008 02:07 CDT
====================================================================== 
Summary:                    REGRESSION: chan_iax2.c patch 122259 results in
chan_iax2.c sending no PING or LAGRQs
Description: 
In latest (124182) 1.4 branch, chan_iax2.c no longer sends any PING or
LAGRQ frames.

It looks like the reason is the change between revs 120168 and 122259,
which has send_ping and send_lagrq reset the pingid and lagid to -1.  

The logged reason for the change was to "Fix some race conditions that
cause ast_assert() to report that chan_iax2 tried to remove an entry that
wasn't in the scheduler"

But if you look at the __send_ping and send_ping taken together
(appended), you can see that once pingid is -1, no more pings will ever get
sent.

(Strange use of a while and break, BTW.  What's wrong with an ordinary
if?)

This change probably got rid of the warning, but it did it by stopping all
PING/LAGRQ.  I think it should be reverted, and instead just check for -1
before ast_sched_delling the pingid/laqid

----------------------------------------------
static int send_ping(const void *data);

static void __send_ping(const void *data)
{
        int callno = (long) data;

        ast_mutex_lock(&iaxsl[callno]);

        while (iaxs[callno] && iaxs[callno]->pingid != -1) {
                if (iaxs[callno]->peercallno) {
                        send_command(iaxs[callno], AST_FRAME_IAX,
IAX_COMMAND_PING, 0, NULL, 0, -1);
                }
                iaxs[callno]->pingid = iax2_sched_add(sched, ping_time *
1000, send_ping, data);
                break;
        }

        ast_mutex_unlock(&iaxsl[callno]);
}

static int send_ping(const void *data)
{
        int callno = (long) data;

        ast_mutex_lock(&iaxsl[callno]);
        if (iaxs[callno]) {
                iaxs[callno]->pingid = -1;
        }
        ast_mutex_unlock(&iaxsl[callno]);

#ifdef SCHED_MULTITHREADED
        if (schedule_action(__send_ping, data))
#endif
                __send_ping(data);

        return 0;
}
----------------------------------------------




====================================================================== 

---------------------------------------------------------------------- 
 mike240se - 06-27-08 02:07  
---------------------------------------------------------------------- 
does this also fix the hanging CLI?  Cause those of us with the iax2 issue
in 1.4.21 also have the CLI hang and then the only way to kill it is to
kill -9 asterisk. 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
06-27-08 02:07  mike240se      Note Added: 0089342                          
======================================================================