[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]

The following issue has been RESOLVED. 
====================================================================== 
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:                     resolved
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:              
Resolution:                 fixed
Fixed in Version:           
====================================================================== 
Date Submitted:             06-20-2008 08:05 CDT
Last Modified:              07-01-2008 13:45 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;
}
----------------------------------------------




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

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
07-01-08 13:45  svnbot         Status                   ready for testing =>
resolved
07-01-08 13:45  svnbot         Resolution               open => fixed       
======================================================================