Re: [asterisk-dev] CentOS 6 and Ubuntu 12 Testing Support
After testing with CentOS 6.8, I agree that it has become difficult to continue supporting it. On Wed, Apr 12, 2017 at 9:27 AM, George Joseph <gjos...@digium.com> wrote: > > > On Wed, Apr 12, 2017 at 7:22 AM, Scott Griepentrog < > sgriepent...@digium.com> wrote: > >> I agree that 12.04 is old enough to not worry about supporting. Because >> of the widespread use of CentOS 6 as an Asterisk platform, I'd be concerned >> about abandoning it however. Which tests (beyond ODBC) are you having >> trouble with? >> > > Pretty much every rest_api test fails because of Python package issues. > The last errors are related to txaio and twisted. > The latest version of twisted to support python 2.6 is 14 which starts a > cascade of package interdependencies that can only be resolved by using pip > to install specific versions of things. I could probably get it to work > but is it worth the time and effort to do so? > > These are the packages that have to be installed via pip just to get this > far... > alembic > setuptools > requests > pyparsing > urlparse3 > urllib3 > virtualenv > virtualenv-clone > virtualenvwrapper > construct > pep8 > autobahn > service-identity > construct==2.5.1 > Twisted==14 > > Oh yeah, one of the packages we install from source (I think it's sipp but > I forget) requires autoconf268 which is available via yum but it's a pain > to get it to be used. > > >> >> On Wed, Apr 12, 2017 at 7:45 AM, Joshua Colp <jc...@digium.com> wrote: >> >>> On Tue, Apr 11, 2017, at 03:28 PM, George Joseph wrote: >>> > Both CentOS 6 and Ubuntu 12 have fallen into the state where we can't >>> > actually create a new instance of either that can run the Asterisk >>> > Testsuite. In order to get it to work I've had to fiddle Python >>> packages >>> > both from the distributions' repositories and directly from pip which >>> > makes >>> > the Python environment fragile FrankenSnake. Also, ODBC packages from >>> > that >>> > era are unreliable so I've had to download and install both UnixODBC >>> and >>> > the postgresql ODBC drivers from source to get a working realtime >>> setup. >>> > Finally, the Ubuntu 12 ISO images contain an /etc/apt/sources.list that >>> > no >>> > longer works right out of the box. >>> > >>> > So what do you folks think the future of testing on CentOS 6 and Ubuntu >>> > 12 >>> > should be? >>> >>> 12.04 is EOL so not testing it is fine to me. CentOS 6 I don't really >>> have a comment on, I'm not in that ecosystem myself. >>> >>> -- >>> Joshua Colp >>> Digium, Inc. | Senior Software Developer >>> 445 Jan Davis Drive NW - Huntsville, AL 35806 - US >>> Check us out at: www.digium.com & www.asterisk.org >>> >>> -- >>> _ >>> -- Bandwidth and Colocation Provided by http://www.api-digital.com -- >>> >>> asterisk-dev mailing list >>> To UNSUBSCRIBE or update options visit: >>>http://lists.digium.com/mailman/listinfo/asterisk-dev >>> >> >> >> >> -- >> [image: Digium logo] >> Scott Griepentrog >> Digium, Inc · Software Developer >> 445 Jan Davis Drive NW · Huntsville, AL 35806 · US >> direct/fax: +1 256 428 6239 · mobile: +1 256 580 6090 >> Check us out at: http://digium.com · http://asterisk.org >> >> -- >> _ >> -- Bandwidth and Colocation Provided by http://www.api-digital.com -- >> >> asterisk-dev mailing list >> To UNSUBSCRIBE or update options visit: >>http://lists.digium.com/mailman/listinfo/asterisk-dev >> > > > > -- > George Joseph > Digium, Inc. | Software Developer > 445 Jan Davis Drive NW - Huntsville, AL 35806 - US > Check us out at: www.digium.com & www.asterisk.org > > > -- > _ > -- Bandwidth and Colocation Provided by http://www.api-digital.com -- > > asterisk-dev mailing list > To UNSUBSCRIBE or update options visit: >http://lists.digium.com/mailman/listinfo/asterisk-dev > -- [image: Digium logo] Scott Griepentrog Digium, Inc · Software Developer 445 Jan Davis Drive NW · Huntsville, AL 35806 · US direct/fax: +1 256 428 6239 · mobile: +1 256 580 6090 Check us out at: http://digium.com · http://asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] CentOS 6 and Ubuntu 12 Testing Support
I agree that 12.04 is old enough to not worry about supporting. Because of the widespread use of CentOS 6 as an Asterisk platform, I'd be concerned about abandoning it however. Which tests (beyond ODBC) are you having trouble with? On Wed, Apr 12, 2017 at 7:45 AM, Joshua Colp <jc...@digium.com> wrote: > On Tue, Apr 11, 2017, at 03:28 PM, George Joseph wrote: > > Both CentOS 6 and Ubuntu 12 have fallen into the state where we can't > > actually create a new instance of either that can run the Asterisk > > Testsuite. In order to get it to work I've had to fiddle Python packages > > both from the distributions' repositories and directly from pip which > > makes > > the Python environment fragile FrankenSnake. Also, ODBC packages from > > that > > era are unreliable so I've had to download and install both UnixODBC and > > the postgresql ODBC drivers from source to get a working realtime setup. > > Finally, the Ubuntu 12 ISO images contain an /etc/apt/sources.list that > > no > > longer works right out of the box. > > > > So what do you folks think the future of testing on CentOS 6 and Ubuntu > > 12 > > should be? > > 12.04 is EOL so not testing it is fine to me. CentOS 6 I don't really > have a comment on, I'm not in that ecosystem myself. > > -- > Joshua Colp > Digium, Inc. | Senior Software Developer > 445 Jan Davis Drive NW - Huntsville, AL 35806 - US > Check us out at: www.digium.com & www.asterisk.org > > -- > _ > -- Bandwidth and Colocation Provided by http://www.api-digital.com -- > > asterisk-dev mailing list > To UNSUBSCRIBE or update options visit: >http://lists.digium.com/mailman/listinfo/asterisk-dev > -- [image: Digium logo] Scott Griepentrog Digium, Inc · Software Developer 445 Jan Davis Drive NW · Huntsville, AL 35806 · US direct/fax: +1 256 428 6239 · mobile: +1 256 580 6090 Check us out at: http://digium.com · http://asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] BuildSystem: make --jobs without amount of thread hangs
I tested running -j without a process limit and while it was successful building 13 on my primary desktop (8g ram 4 core machine with ssd), it resulted in a load average over 40, sluggish response, and ran slightly slower than -j$(nproc). Here is my results: 1m32.204s => make -j 1m27.274s => make -j$(nproc) 4m50.423s => make With a lessor powered system (1g ram, 1 cpu): During make -j the system became extremely sluggish after the first few minutes, with a load avg over 600 and over 1200 processes in running state. After about 20 minutes I gave up waiting for it to finish. 8m16.065s => make -j$(nproc) If the high number of processes with -j starts eating enough virtual memory that the page rate becomes significant, the system performance slows to a crawl making the entire system unusable. There is no problem in either make or Asterisk build process as far as I am able to see. On Thu, Jun 9, 2016 at 5:16 AM, Tzafrir Cohen <tzafrir.co...@xorcom.com> wrote: > On Wed, Jun 08, 2016 at 02:09:04PM -0500, Matt Fredrickson wrote: > > > I usually don't build Asterisk with multiple jobs enabled, so I'm not > > sure if we support this or not. > > IIRC it mostly work, but only as long as you don't mix 'make' and 'make > install'. IIRC this is commonly used by packagers. But I may be wrong. > > -- >Tzafrir Cohen > icq#16849755 jabber:tzafrir.co...@xorcom.com > +972-50-7952406 mailto:tzafrir.co...@xorcom.com > http://www.xorcom.com > > -- > _ > -- Bandwidth and Colocation Provided by http://www.api-digital.com -- > > asterisk-dev mailing list > To UNSUBSCRIBE or update options visit: > http://lists.digium.com/mailman/listinfo/asterisk-dev > -- [image: Digium logo] Scott Griepentrog Digium, Inc · Software Developer 445 Jan Davis Drive NW · Huntsville, AL 35806 · US direct/fax: +1 256 428 6239 · mobile: +1 256 580 6090 Check us out at: http://digium.com · http://asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] BuildSystem: make --jobs without amount of thread hangs
I would suspect that to be a problem with make dealing successfully with the omitted number. I use this all the time without incident: make -j$(nproc) On Wed, Jun 8, 2016 at 1:58 PM, Alexander Traud <pabstr...@compuserve.com> wrote: > On Ubuntu 16.04 LTS, when I build Asterisk 13.9.1 via > > > wget > downloads.asterisk.org/pub/telephony/asterisk/asterisk-13-current.tar.gz > tar zxf asterisk-1*.tar.gz > sudo apt install libssl-dev libncurses-dev libnewt-dev libxml2-dev > libsqlite3-dev uuid-dev libjansson-dev libedit-dev libblocksruntime-dev > ./configure > > make --jobs > > the whole computer hangs. Normally, this indicates a dependency issue in > the Makefile. Furthermore, I faced dependency issues with make --jobs while > doing a partial compile. However, at that time, I did not debug this. > Therefore, even specifying not unlimited but a certain amount of threads > might not reliable in all cases. > > I would love to debug this, however, I have no idea where to start. Does > anyone know about a debugging strategy for this? Or should I just report > this issue on JIRA, so that someone else looks into this one day? > > > -- > _ > -- Bandwidth and Colocation Provided by http://www.api-digital.com -- > > asterisk-dev mailing list > To UNSUBSCRIBE or update options visit: >http://lists.digium.com/mailman/listinfo/asterisk-dev > -- [image: Digium logo] Scott Griepentrog Digium, Inc · Software Developer 445 Jan Davis Drive NW · Huntsville, AL 35806 · US direct/fax: +1 256 428 6239 · mobile: +1 256 580 6090 Check us out at: http://digium.com · http://asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] ast_safe_system() from within chan_sip.c
> unsuccessful register count Why not use the security event logging system instead? Would be easier than modifying chan_sip, and less prone to introducing undesired side affects by inserting a system call. https://wiki.asterisk.org/wiki/display/AST/Asterisk+Security+Framework On Mon, Feb 22, 2016 at 10:06 AM, Pedro Guillem [e-Mediate] <p...@e-mediate.co > wrote: > Hi Folks > > I haven´t used C in a decade so i´m kind of rusty, but I understand the > advanced basics of it. > > > > I know there is a function called ast_safe_system() declared on asterisk.c > used to perform system calls. > > I need chan_sip.c to call a local program each time a registration is > unsuccessful. > > > > I know the exact line of chan_sip.c where I can trigger this system call, > but I don´t know if I can use ast_safe_system() from within a function of > chan_sip.c. > > > > Any ideas? > > Should I include an additional header files to chan_sip.c in order to call > this function? > > > > Best > > Pedro > > > > PS: Yeah.. most of you will probably ask why on earth would I want to do > that.. > > Quick answer is I don´t want to modify chan_sip.c that much in order to do > something like keeping an unsuccessful register count… so i´d rather > delegate that to a third party script > > > > > > > > > > Este correo electrónico se ha enviado desde un equipo libre de virus y > protegido por Avast. > www.avast.com <https://www.avast.com/sig-email> > > -- > _ > -- Bandwidth and Colocation Provided by http://www.api-digital.com -- > > asterisk-dev mailing list > To UNSUBSCRIBE or update options visit: >http://lists.digium.com/mailman/listinfo/asterisk-dev > -- [image: Digium logo] Scott Griepentrog Digium, Inc · Software Developer 445 Jan Davis Drive NW · Huntsville, AL 35806 · US direct/fax: +1 256 428 6239 · mobile: +1 256 580 6090 Check us out at: http://digium.com · http://asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] asterisk 11.18.0-rc1 Now Available
Although I would recommend switching to git instead, it is possible to use svn co on the github repository as such: svn co https://github.com/asterisk/asterisk/branches/13 On Fri, May 22, 2015 at 6:14 AM, Joshua Colp jc...@digium.com wrote: Pavel Troller wrote: Hi! Kia ora, Not a single change on the SVN. So, is SVN totally abandoned even for reading and git is the only way to keep track ? Hmmm... That is correct. There is no synchronization back to SVN. If it's true, could anybody post a few points, how to convert from SVN to git ? At least an URL would be great. On asterisk.org, links are still pointing to the SVN :-(. There's a few ways you can get the git repository and browse. If you'd like to clone straight from the source then gerrit is where to go. This is also how code reviews are submitted: https://gerrit.asterisk.org/#/admin/projects/asterisk The repository is also mirrored onto github for browsing and read-only clones: https://github.com/asterisk/asterisk/ I also do agree that stuff still needs to be updated. We'll get there! Cheers, -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev -- [image: Digium logo] Scott Griepentrog Digium, Inc · Software Developer 445 Jan Davis Drive NW · Huntsville, AL 35806 · US direct/fax: +1 256 428 6239 · mobile: +1 256 580 6090 Check us out at: http://digium.com · http://asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] asterisk 11.18.0-rc1 Now Available
The github branch is a read-only clone of our official repository. Anything you want put into the Asterisk project must go through our own git (only) server at gerrit.asterisk.org. So, no, the only thing that you can do from github is clone a copy of the repository for your private use. On Fri, May 22, 2015 at 7:26 AM, Kaloyan Kovachev kkovac...@varna.net wrote: Will 'svn relocate' work too? And will it continue from where it left? I have some modifications to the code on a checkout from branch 11 and would like to keep it up to date without reapplying the patches to new checkout On 2015-05-22 15:08, Scott Griepentrog wrote: Although I would recommend switching to git instead, it is possible to use svn co on the github repository as such: svn co https://github.com/asterisk/asterisk/branches/13 [1] On Fri, May 22, 2015 at 6:14 AM, Joshua Colp jc...@digium.com wrote: Pavel Troller wrote: Hi! Kia ora, Not a single change on the SVN. So, is SVN totally abandoned even for reading and git is the only way to keep track ? Hmmm... That is correct. There is no synchronization back to SVN. If it's true, could anybody post a few points, how to convert from SVN to git ? At least an URL would be great. On asterisk.org [2], links are still pointing to the SVN :-(. There's a few ways you can get the git repository and browse. If you'd like to clone straight from the source then gerrit is where to go. This is also how code reviews are submitted: https://gerrit.asterisk.org/#/admin/projects/asterisk [3] The repository is also mirrored onto github for browsing and read-only clones: https://github.com/asterisk/asterisk/ [4] I also do agree that stuff still needs to be updated. We'll get there! Cheers, -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com [5] www.asterisk.org [6] -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com [7] -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev [8] -- Scott Griepentrog Digium, Inc · Software Developer 445 Jan Davis Drive NW · Huntsville, AL 35806 · US direct/fax: +1 256 428 6239 · mobile: +1 256 580 6090 Check us out at: http://digium.com [9] · http://asterisk.org [2] Links: -- [1] https://github.com/asterisk/asterisk/branches/13 [2] http://asterisk.org [3] https://gerrit.asterisk.org/#/admin/projects/asterisk [4] https://github.com/asterisk/asterisk/ [5] http://www.digium.com [6] http://www.asterisk.org [7] http://www.api-digital.com [8] http://lists.digium.com/mailman/listinfo/asterisk-dev [9] http://digium.com -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev -- [image: Digium logo] Scott Griepentrog Digium, Inc · Software Developer 445 Jan Davis Drive NW · Huntsville, AL 35806 · US direct/fax: +1 256 428 6239 · mobile: +1 256 580 6090 Check us out at: http://digium.com · http://asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Asterisk Beacon Module Proposal
Correction: as one will end up getting a new ID (the second one to reuse the same token). On Tue, May 12, 2015 at 1:24 PM, Scott Griepentrog sgriepent...@digium.com wrote: So as opposed to spoofing, there is also the case that someone having a copy of Asteirsk in a virtual machine clones it, and ends up with two instances reporting the same random ID. With the spoofing detection mechanism (using tokens to get an ID from the server), the effect of this case is minimized as each one will end up getting a new ID after token timeout. On Tue, May 12, 2015 at 12:59 PM, Tzafrir Cohen tzafrir.co...@xorcom.com wrote: Quoting the spec: | Spoofing | | In order to limit spoofing, the server will return a token for all | accepted requests to a server. Any subsequent requests to that resource | must present the token in the request. If a subsequent request fails to | provide the token, the request is rejected. Tokens expire after 48 | hours, at which point, a request does not have to provide a token. If a | request does provide a token that is expired - and no token is required | at that point - the request should be accepted and a new token granted. | Once a request is made without a token (and no token is expected), a new | token is issued for subsequent requests. | | So long as Asterisk's transmission of data occurs faster than once every | 48 hours, a malicious entity will not be able to spoof a resource. If a | system is down then a remote system can 'take over' a system, and the | legitimate system's attempts will be rejected. If that occurs... oh | well. It is anonymous data. I'm not sure I understand the need for the token. The Debian popularity-contest (popcon, [1]) only identifies systems by a single random token (MY_HOSTID in /etc/popularity-contest.conf). It supports sending information by mail as well (thus: completely non-interactively). I don't see what the extra temporary token buys here. Just send a report that includes the (random) server ID. Nobody should be able to copy those (as they are only sent encrypted over the internet). And in any event, why would anybody want to spoof that (as opposed to merely add records to skew the stats, which is possible either way just as easily). What am I missing here? [1] https://packages.debian.org/sid/popularity-contest -- Tzafrir Cohen icq#16849755 jabber:tzafrir.co...@xorcom.com +972-50-7952406 mailto:tzafrir.co...@xorcom.com http://www.xorcom.com -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev -- [image: Digium logo] Scott Griepentrog Digium, Inc · Software Developer 445 Jan Davis Drive NW · Huntsville, AL 35806 · US direct/fax: +1 256 428 6239 · mobile: +1 256 580 6090 Check us out at: http://digium.com · http://asterisk.org -- [image: Digium logo] Scott Griepentrog Digium, Inc · Software Developer 445 Jan Davis Drive NW · Huntsville, AL 35806 · US direct/fax: +1 256 428 6239 · mobile: +1 256 580 6090 Check us out at: http://digium.com · http://asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Asterisk Beacon Module Proposal
So as opposed to spoofing, there is also the case that someone having a copy of Asteirsk in a virtual machine clones it, and ends up with two instances reporting the same random ID. With the spoofing detection mechanism (using tokens to get an ID from the server), the effect of this case is minimized as each one will end up getting a new ID after token timeout. On Tue, May 12, 2015 at 12:59 PM, Tzafrir Cohen tzafrir.co...@xorcom.com wrote: Quoting the spec: | Spoofing | | In order to limit spoofing, the server will return a token for all | accepted requests to a server. Any subsequent requests to that resource | must present the token in the request. If a subsequent request fails to | provide the token, the request is rejected. Tokens expire after 48 | hours, at which point, a request does not have to provide a token. If a | request does provide a token that is expired - and no token is required | at that point - the request should be accepted and a new token granted. | Once a request is made without a token (and no token is expected), a new | token is issued for subsequent requests. | | So long as Asterisk's transmission of data occurs faster than once every | 48 hours, a malicious entity will not be able to spoof a resource. If a | system is down then a remote system can 'take over' a system, and the | legitimate system's attempts will be rejected. If that occurs... oh | well. It is anonymous data. I'm not sure I understand the need for the token. The Debian popularity-contest (popcon, [1]) only identifies systems by a single random token (MY_HOSTID in /etc/popularity-contest.conf). It supports sending information by mail as well (thus: completely non-interactively). I don't see what the extra temporary token buys here. Just send a report that includes the (random) server ID. Nobody should be able to copy those (as they are only sent encrypted over the internet). And in any event, why would anybody want to spoof that (as opposed to merely add records to skew the stats, which is possible either way just as easily). What am I missing here? [1] https://packages.debian.org/sid/popularity-contest -- Tzafrir Cohen icq#16849755 jabber:tzafrir.co...@xorcom.com +972-50-7952406 mailto:tzafrir.co...@xorcom.com http://www.xorcom.com -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev -- [image: Digium logo] Scott Griepentrog Digium, Inc · Software Developer 445 Jan Davis Drive NW · Huntsville, AL 35806 · US direct/fax: +1 256 428 6239 · mobile: +1 256 580 6090 Check us out at: http://digium.com · http://asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Review Request: Sorcery Caching
+1 On Sat, May 2, 2015 at 7:08 AM, Joshua Colp jc...@digium.com wrote: Greetings all, Based on the feedback I've updated the wiki page[1] with tweaks, additional potential tests, clarification, etc. Cheers, [1] https://wiki.asterisk.org/wiki/display/~jcolp/Sorcery+Caching -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev -- [image: Digium logo] Scott Griepentrog Digium, Inc · Software Developer 445 Jan Davis Drive NW · Huntsville, AL 35806 · US direct/fax: +1 256 428 6239 · mobile: +1 256 580 6090 Check us out at: http://digium.com · http://asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Review Request: Sorcery Caching
On the subject of maxiumum cache size - I like being able to limit the memory usage, but there is apparently no way to prioritize often used entries. I'm thinking of an example where my cache is full, having gone through every entry, but then a small subset of entries is frequently read which may not have been the first into the cache. If I understand the wording of the wiki article correctly, these would then not benefit from caching, since they would not be allowed in the cache? Also, I'm unclear on the differences between the three object expire/lifetime values. Can you add more descriptive details to each of these values, possibly give an example showing why it is beneficial? My thoughts on cache needs are this: 1) I want to be able to set the maximum memory dedicated to caching, either in MB or in # of entries, but have the caching algorithm take into account the most recently read entries when determining what to hold onto and what to throw away. Something I've just looked up is more likely to used again than something that has been sitting in the cache for a while. To accomplish this each entry needs a timestamp of the last cache hit (when it was last used/read). 2) I want to be able to set two maximum time in cache values: The first is the seconds after which the entry is stale, and the second where it is guarunteed to be removed from the cache and no longer used. Both values are from the time the entry was last obtained from the source (db), requiring a timestamp on entry creation or when it was last refreshed from the source. Before the entry is stale, it can be re-used without any further action needed. When a stale entry is accessed (the first time) it should trigger a background refresh to ensure that changes have not been made, but continue to return a hit from the cache until the second timeout. Background refreshes to be initiated by a separate thread only when there are no primary requests active. The idea here is I can set 300 seconds for stale, and 600 for lifetime -- for a frequently used entry I'm only refreshing from the database every 2.5 minutes and 100% of the requests come immediately from the cache with no database lookup delay to the requesting thread. For any entry that hasn't been read in 5 minutes, it's deleted so the memory utilization goes away also. 3) Flushing all entries on a periodic interval doesn't sound like an option I'd be interested in -- assuming that a maximum time in cache is enforced properly, this seems like an odd duplication. My worry is that dumping all the entries will occasionally occur in the middle of a burst of reads, and thus significantly slow things down. 4) The external notification should be able to forcibly empty the cache, or optionally instead just force all objects to be stale without removing them. This would require adding a stale flag to the entry. 5) The rebalancing option sounds like a good idea, but since it is such an implementation specific value, it would be useful to run some tests and document some statistics on what conditions are needed for it to go from negligable to noticeable improvement, so that people have an idea of when and how to use it. On Tue, Apr 28, 2015 at 11:28 AM, Joshua Colp jc...@digium.com wrote: Kia ora, I've created a wiki page[1] which details the beginnings of a basic memory based caching wizard for sorcery. Right now while caching is possible using the existing memory wizard it's not possible to define object lifetimes, so once cached it's always pulled from the cache. This wiki page uses the memory wizard as a base but defines options which can tweak the behavior. Going forward this could serve as a basis for other wizards to be created for caching purposes. Some things to consider: 1. How much control and flexibility should we allow? 2. Are there additional mechanisms that should be exposed to allow explicit object expiration? 3. Are the defaults sane? 4. Is there additional testing that should be done? 5. Does anything need additional explanation? Cheers, [1] https://wiki.asterisk.org/wiki/display/~jcolp/Sorcery+Caching -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev -- [image: Digium logo] Scott Griepentrog Digium, Inc · Software Developer 445 Jan Davis Drive NW · Huntsville, AL 35806 · US direct/fax: +1 256 428 6239 · mobile: +1 256 580 6090 Check us out at: http://digium.com · http://asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list
Re: [asterisk-dev] [Code Review] 4595: Voicemail API: fix handling of mailbox full condition
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4595/ --- (Updated April 7, 2015, 2:34 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 434260 Bugs: ASTERISK-24942 https://issues.asterisk.org/jira/browse/ASTERISK-24942 Repository: Asterisk Description --- In r115582 (2008), the ERROR_MAILBOX_FULL flag was removed, and a check of the save_to_folder result no longer handled the mailbox full condition. This lead to the deletion of a new Inbox message, once played, that could not be relocated to the Old mailbox because of the maxmsg limit. This patch restores the original functionality lost, which is to leave the message in the Inbox without deleting it. Diffs - /branches/11/apps/app_voicemail.c 434135 Diff: https://reviewboard.asterisk.org/r/4595/diff/ Testing --- Tested manually on my system under Asterisk 13. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4574: pjsip: resolve compatibility problem with ast_sip_session struct
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4574/ --- (Updated April 2, 2015, 9:56 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 433944 Repository: Asterisk Description --- A compatibility problem was introduced in r430179 (https://reviewboard.asterisk.org/r/4308) when a new variable was inserted into the struct ast_sip_session. This change puts that new variable at the end of the structure to avoid misalignment of the values. This also affects releases 13.2 and 13.3. Diffs - /branches/13/include/asterisk/res_pjsip_session.h 433918 Diff: https://reviewboard.asterisk.org/r/4574/diff/ Testing --- With this change, a DPMA crash has been eliminated when transmitting messages via PJSIP. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Problems Building 13.x with GCC 5.0
Have you submitted a ticket on http://issues.asterisk.org ? On Wed, Apr 1, 2015 at 2:50 PM, Jeffrey Ollie j...@ocjtech.us wrote: Fedora recently switched to GCC 5.0 as the default C compiler, and Asterisk 13.X stopped building. It was first brought to my attention with 13.1.1, but 13.3.0 won't compile either. https://bugzilla.redhat.com/show_bug.cgi?id=1195304 https://kojipkgs.fedoraproject.org//work/tasks/4722/9394722/build.log Can anyone suggest a fix? I'd like to get the Fedora packages updated. -- Jeff Ollie -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev -- [image: Digium logo] Scott Griepentrog Digium, Inc · Software Developer 445 Jan Davis Drive NW · Huntsville, AL 35806 · US direct/fax: +1 256 428 6239 · mobile: +1 256 580 6090 Check us out at: http://digium.com · http://asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4574: pjsip: resolve compatibility problem with ast_sip_session struct
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4574/ --- (Updated April 1, 2015, 4:20 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description (updated) --- A compatibility problem was introduced in r430179 (https://reviewboard.asterisk.org/r/4308) when a new variable was inserted into the struct ast_sip_session. This change puts that new variable at the end of the structure to avoid misalignment of the values. This also affects releases 13.2 and 13.3. Diffs - /branches/13/include/asterisk/res_pjsip_session.h 433918 Diff: https://reviewboard.asterisk.org/r/4574/diff/ Testing --- With this change, a DPMA crash has been eliminated when transmitting messages via PJSIP. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4463: core: Introduce chaos into memory allocations
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4463/ --- (Updated March 17, 2015, 4:57 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 433060 Repository: Asterisk Description --- Introduce chaotic (random) failures into certain critical operations to force improvements to error handling. This patch introduces the DEBUG_CHAOS random failure mechanism and adds it to memory allocation wrappers in utils.h. To be activated, DEBUG_CHAOS must be enabled in menuselect. The failure rate (1 in X) is controlled by changing the define DEBUG_CHAOS_CHANCES_1IN in utils.h. Diffs - /branches/13/include/asterisk/utils.h 432661 /branches/13/build_tools/cflags.xml 432661 Diff: https://reviewboard.asterisk.org/r/4463/diff/ Testing --- I'm unable to get Asterisk to actually start with 1 in 100,000 failure rate. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4468: Various: bugfixes discovered through use of chaos
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4468/ --- (Updated March 17, 2015, 5:15 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 433064 Repository: Asterisk Description --- Using the CHAOS random failure mechanism (https://reviewboard.asterisk.org/r/4463/) I uncovered several possibilities of a crash on null pointer, and one uninitialized variable that allowed ast_random to lock up if used before utils init. Additionally, extra output to determine why Asterisk failed to initialize has been added. Diffs - /branches/13/main/xmldoc.c 432661 /branches/13/main/utils.c 432661 /branches/13/main/manager.c 432661 /branches/13/main/endpoints.c 432661 /branches/13/main/config.c 432661 /branches/13/main/codec_builtin.c 432661 /branches/13/main/asterisk.c 432661 /branches/13/include/asterisk/config.h 432661 Diff: https://reviewboard.asterisk.org/r/4468/diff/ Testing --- Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4497: logger: init_logger_chain has unreachable code
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4497/#review14725 --- Ship it! Ship It! - Scott Griepentrog On March 14, 2015, 4:26 p.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4497/ --- (Updated March 14, 2015, 4:26 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24817 https://issues.asterisk.org/jira/browse/ASTERISK-24817 Repository: Asterisk Description --- init_logger_chain has a block of code to set a default console logger if logger.conf was invalid or not found. Since the code is unreachable, no logger channels to be created. This changes it so the default console logger is applied any time logger.conf load fails. Diffs - /branches/11/main/logger.c 432991 Diff: https://reviewboard.asterisk.org/r/4497/diff/ Testing --- Verified that nothing was logged to console with missing logger.conf, and that this patch caused console logging to be enabled in that case. Also verified that valid logger.conf was not effected. Thanks, Corey Farrell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4468: Various: bugfixes discovered through use of chaos
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4468/ --- (Updated March 10, 2015, 10:27 a.m.) Review request for Asterisk Developers. Changes --- Addressed review feedback. Repository: Asterisk Description --- Using the CHAOS random failure mechanism (https://reviewboard.asterisk.org/r/4463/) I uncovered several possibilities of a crash on null pointer, and one uninitialized variable that allowed ast_random to lock up if used before utils init. Additionally, extra output to determine why Asterisk failed to initialize has been added. Diffs (updated) - /branches/13/main/xmldoc.c 432661 /branches/13/main/utils.c 432661 /branches/13/main/manager.c 432661 /branches/13/main/endpoints.c 432661 /branches/13/main/config.c 432661 /branches/13/main/codec_builtin.c 432661 /branches/13/main/asterisk.c 432661 /branches/13/include/asterisk/config.h 432661 Diff: https://reviewboard.asterisk.org/r/4468/diff/ Testing --- Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4463: core: Introduce chaos into memory allocations
On March 9, 2015, 8:19 a.m., Corey Farrell wrote: /branches/13/main/utils.c, line 614 https://reviewboard.asterisk.org/r/4463/diff/2/?file=71877#file71877line614 This looks like it should be applied to 11 as well. Joshua Colp wrote: I'd go so far as to say that bug fixes as a result of this should be a separate review. I'll split out the bug fixes as a separate review. - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4463/#review14613 --- On March 7, 2015, 12:01 a.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4463/ --- (Updated March 7, 2015, 12:01 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Introduce chaotic (random) failures into certain critical operations to force improvements to error handling. This patch introduces the DEBUG_CHAOS random failure mechanism and adds it to memory allocation wrappers in utils.h. To be activated, DEBUG_CHAOS must be enabled in menuselect. The failure rate (1 in X) is controlled by changing the define DEBUG_CHAOS_CHANCES_1IN in utils.h. Diffs - /branches/13/main/xmldoc.c 432613 /branches/13/main/utils.c 432613 /branches/13/main/endpoints.c 432613 /branches/13/main/config.c 432613 /branches/13/main/codec_builtin.c 432613 /branches/13/main/asterisk.c 432613 /branches/13/include/asterisk/utils.h 432613 /branches/13/build_tools/cflags.xml 432613 Diff: https://reviewboard.asterisk.org/r/4463/diff/ Testing --- I'm unable to get Asterisk to actually start with 1 in 100,000 failure rate. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4463: core: Introduce chaos into memory allocations
On March 9, 2015, 7:41 a.m., Joshua Colp wrote: Do you plan on other chaotic things? As it is the chaos is really just memory allocation failures, nothing outside the scope of that. Yes -- the idea is that chaos could be easily inserted into any function that has a specific failure return value. The purpose being to expose extremely uncommon code paths, by introducing a higher chance of execution than practically zero. The memory allocation functions are the easiest and first target for this. However, use of chaos elsewhere could end up being more of a personal development testing feature than something that is checked into the official code. - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4463/#review14610 --- On March 7, 2015, 12:01 a.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4463/ --- (Updated March 7, 2015, 12:01 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Introduce chaotic (random) failures into certain critical operations to force improvements to error handling. This patch introduces the DEBUG_CHAOS random failure mechanism and adds it to memory allocation wrappers in utils.h. To be activated, DEBUG_CHAOS must be enabled in menuselect. The failure rate (1 in X) is controlled by changing the define DEBUG_CHAOS_CHANCES_1IN in utils.h. Diffs - /branches/13/main/xmldoc.c 432613 /branches/13/main/utils.c 432613 /branches/13/main/endpoints.c 432613 /branches/13/main/config.c 432613 /branches/13/main/codec_builtin.c 432613 /branches/13/main/asterisk.c 432613 /branches/13/include/asterisk/utils.h 432613 /branches/13/build_tools/cflags.xml 432613 Diff: https://reviewboard.asterisk.org/r/4463/diff/ Testing --- I'm unable to get Asterisk to actually start with 1 in 100,000 failure rate. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4468: Various: bugfixes discovered through use of chaos
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4468/ --- (Updated March 9, 2015, 3:50 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description (updated) --- Using the CHAOS random failure mechanism (https://reviewboard.asterisk.org/r/4463/) I uncovered several possibilities of a crash on null pointer, and one uninitialized variable that allowed ast_random to lock up if used before utils init. Additionally, extra output to determine why Asterisk failed to initialize has been added. Diffs - /branches/13/main/xmldoc.c 432661 /branches/13/main/utils.c 432661 /branches/13/main/endpoints.c 432661 /branches/13/main/config.c 432661 /branches/13/main/codec_builtin.c 432661 /branches/13/main/asterisk.c 432661 Diff: https://reviewboard.asterisk.org/r/4468/diff/ Testing --- Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 4468: Various: bugfixes discovered through use of chaos
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4468/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- Using the CHAOS random failure mechanism (https://reviewboard.asterisk.org/r/4463/) I uncovered several possibilities of a crash on null pointer, and one uninitialized variable that allowed ast_random to lock up if used before utils init. Diffs - /branches/13/main/xmldoc.c 432661 /branches/13/main/utils.c 432661 /branches/13/main/endpoints.c 432661 /branches/13/main/config.c 432661 /branches/13/main/codec_builtin.c 432661 /branches/13/main/asterisk.c 432661 Diff: https://reviewboard.asterisk.org/r/4468/diff/ Testing --- Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 4463: core: Introduce chaos into memory allocations
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4463/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- Introduce chaotic (random) failures into certain critical operations to force improvements to error handling. This patch introduces the DEBUG_CHAOS random failure mechanism and adds it to memory allocation wrappers in utils.h. To be activated, DEBUG_CHAOS must be enabled in menuselect. The failure rate (1 in X) is controlled by changing the define DEBUG_CHAOS_CHANCES_1IN in utils.h. Diffs - /branches/13/main/utils.c 432572 /branches/13/include/asterisk/utils.h 432572 /branches/13/build_tools/cflags.xml 432572 Diff: https://reviewboard.asterisk.org/r/4463/diff/ Testing --- I'm unable to get Asterisk to actually start with 1 in 100,000 failure rate. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4463: core: Introduce chaos into memory allocations
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4463/ --- (Updated March 7, 2015, 12:01 a.m.) Review request for Asterisk Developers. Changes --- Fixed several crashes on Asterisk startup. Added diagnostic output on initialization failures. Reworked CHAOS slightly to allow for use in other locations with different chance values. Repository: Asterisk Description --- Introduce chaotic (random) failures into certain critical operations to force improvements to error handling. This patch introduces the DEBUG_CHAOS random failure mechanism and adds it to memory allocation wrappers in utils.h. To be activated, DEBUG_CHAOS must be enabled in menuselect. The failure rate (1 in X) is controlled by changing the define DEBUG_CHAOS_CHANCES_1IN in utils.h. Diffs (updated) - /branches/13/main/xmldoc.c 432613 /branches/13/main/utils.c 432613 /branches/13/main/endpoints.c 432613 /branches/13/main/config.c 432613 /branches/13/main/codec_builtin.c 432613 /branches/13/main/asterisk.c 432613 /branches/13/include/asterisk/utils.h 432613 /branches/13/build_tools/cflags.xml 432613 Diff: https://reviewboard.asterisk.org/r/4463/diff/ Testing --- I'm unable to get Asterisk to actually start with 1 in 100,000 failure rate. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4444: chan_dahdi/sig_analog: Fix distinctive ring detection to suck less.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r//#review14593 --- Ship it! Problems with CID detection resolved by swapping out hardware (other random Dahdi failures were being seen). Problem with distinctive ring detection on my TDM400P board is a limitation of either the hardware or the lower level Dahdi driver not being able to detect and/or signal for an OFF ring shorter period of 200ms. DR3 and DR6 work correctly as they have 400ms OFF periods. Effectively, the other DR patterns are signalled as one continuous ring due to use of only 200ms OFF. Since the distictive ring detection issues I'm encountering are not the fault of chan_dahdi, sig_analog, or these improvements, I'm good with this. - Scott Griepentrog On March 2, 2015, 3:18 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r// --- (Updated March 2, 2015, 3:18 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24825 https://issues.asterisk.org/jira/browse/ASTERISK-24825 Repository: Asterisk Description --- The distinctive ring feature interferes with detecting Caller ID and appears to have been broken for years. What happens is if you have a ring-ring cadence as used in the UK you get too many DAHDI events for the distinctive ring pattern array and Caller ID detection is aborted. I think when Zapata/DAHDI added the ring begin event it broke distinctive ring. More events happen than before and the code does no filtering of which event times are recorded in the pattern array. * Made distinctive ring only record the ringt count when the ring ends instead of on just any DAHDI event. Distinctive ring can be ring, ring-ring, ring-ring-ring, or different ring durations for the up to three rings. * Fixed the distinctive ring detection enable (chan_dahdi.conf option usedistinctiveringdetection) to be per port instead of somewhat per port and somewhat global. This has been broken since v1.8. * Fixed using the default distinctive ring context when the detected pattern does not match any configured dringX patterns. The default context did not get set when the previous call was a matched distinctive ring pattern and the current call is not matched. This has been broken since v1.8. * Made distinctive ring have no effect on Caller ID detection when it is disabled. Caller ID detection just monitors for 10 seconds before giving up. * Fixed leak of struct callerid_state memory when a polarity reversal during Caller ID detection causes the incoming call to be aborted. Diffs - /branches/11/channels/sig_analog.c 432422 /branches/11/channels/sig_analog.h 432422 /branches/11/channels/chan_dahdi.c 432422 /branches/11/UPGRADE.txt 432422 Diff: https://reviewboard.asterisk.org/r//diff/ Testing --- Tested the following scenarios for US (ring) and UK (ring-ring) ring cadences: 1) usedistinctiveringdetection=no 2) usedistinctiveringdetection=yes with distinctiveringaftercid=no 3) usedistinctiveringdetection=yes with distinctiveringaftercid=yes * Caller-ID was detected for each call * The configured distinctive ring dringX pattern was detected and the specified context set. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4444: chan_dahdi/sig_analog: Fix distinctive ring detection to suck less.
On Feb. 27, 2015, 9:39 a.m., Scott Griepentrog wrote: I updated the chan_dahdi [channels] configuration with cadences matching the Bellcore spec and retested for both CID detection and DR pattern recognition. Results are certain patterns (especially DR2, DR4, and DR5) exhibit failure rate on CID detection approaching 50%. As the DR1 pattern has a 0% failure rate, I'm confident that it's still a remaining DR problem. Additionally, the DR pattern numbers (#,#,#) are inconsistent for certain cadences. DR2 for example may be 359,297,259, sometimes 359,0,0, sometimes 297,0,0. The inconsistent DR patterns may or may not be related to the CID failure, although there appears to be a correlation. There may be leftover artifacts on CID detection between calls, as repeating a failed call tends to correlate to another failure. Scott Griepentrog wrote: I forgot to paste in my cadences: usedistinctiveringdetection=yes ;Bellcore-r1: ;60(2/4) cadence=2000,-4000 ;Bellcore-r2: ;60(.3/.2,1/.2,.3/4) cadence=300,200,1000,200,300,-4000 ;Bellcore-r3: ;60(.8/.4,.8/4) cadence=800,400,800,-4000 ;Bellcore-r4: ;60(.4/.2,.3/.2,.8/4) cadence=400,200,300,200,800,-4000 ;Bellcore-r5: ;60(.2/.2,.2/.2,.2/.2,1/4) cadence=200,200,200,200,200,200,1000,-4000 ;Bellcore-r6: ;60(.2/.4,.2/.4,.2/4) cadence=200,400,200,400,200,-4000 ;Bellcore-r7: ;60(.4/.2,.4/.2,.4/4) cadence=400,200,400,200,400,-4000 ;Bellcore-r8: ;60(0.25/9.75) cadence=250,-9750 I had an error in my cadence configuration. For the testing above I was actually running: cadence=125,125,2000,-4000 cadence=250,250,500,1000,250,250,500,-4000 cadence=125,125,125,125,125,-4000 cadence=1000,500,2500,-5000 cadence=1000,1000,1000,1000,-4000 cadence=500,500,1000,2000,2000,-4000 Once corrected to proper bellcore set below, I still encountered ~50% CID failure and 75% dring detection failure (i.e. all 3 numbers 0 on patterns dr1). cadence 'r1' added: 2000,-4000 cadence 'r2' added: 300,200,1000,200,300,-4000 cadence 'r3' added: 800,400,800,-4000 cadence 'r4' added: 400,200,300,200,800,-4000 cadence 'r5' added: 200,200,200,200,200,200,1000,-4000 cadence 'r6' added: 200,400,200,400,200,-4000 cadence 'r7' added: 400,200,400,200,400,-4000 cadence 'r8' added: 250,-9750 - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r//#review14562 --- On Feb. 23, 2015, 6:51 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r// --- (Updated Feb. 23, 2015, 6:51 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24825 https://issues.asterisk.org/jira/browse/ASTERISK-24825 Repository: Asterisk Description --- The distinctive ring feature interferes with detecting Caller ID and appears to have been broken for years. What happens is if you have a ring-ring cadence as used in the UK you get too many DAHDI events for the distinctive ring pattern array and Caller ID detection is aborted. I think when Zapata/DAHDI added the ring begin event it broke distinctive ring. More events happen than before and the code does no filtering of which event times are recorded in the pattern array. * Made distinctive ring only record the ringt count when the ring ends instead of on just any DAHDI event. Distinctive ring can be ring, ring-ring, ring-ring-ring, or different ring durations for the up to three rings. * Fixed the distinctive ring detection enable (chan_dahdi.conf option usedistinctiveringdetection) to be per port instead of somewhat per port and somewhat global. This has been broken since v1.8. * Fixed using the default distinctive ring context when the detected pattern does not match any configured dringX patterns. The default context did not get set when the previous call was a matched distinctive ring pattern and the current call is not matched. This has been broken since v1.8. * Made distinctive ring have no effect on Caller ID detection when it is disabled. Caller ID detection just monitors for 10 seconds before giving up. * Fixed leak of struct callerid_state memory when a polarity reversal during Caller ID detection causes the incoming call to be aborted. Diffs - /branches/11/channels/sig_analog.c 432173 /branches/11/channels/sig_analog.h 432173 /branches/11/channels/chan_dahdi.c 432173 /branches/11/UPGRADE.txt 432173 Diff: https://reviewboard.asterisk.org/r//diff/ Testing --- Tested
Re: [asterisk-dev] [Code Review] 4444: chan_dahdi/sig_analog: Fix distinctive ring detection to suck less.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r//#review14562 --- I updated the chan_dahdi [channels] configuration with cadences matching the Bellcore spec and retested for both CID detection and DR pattern recognition. Results are certain patterns (especially DR2, DR4, and DR5) exhibit failure rate on CID detection approaching 50%. As the DR1 pattern has a 0% failure rate, I'm confident that it's still a remaining DR problem. Additionally, the DR pattern numbers (#,#,#) are inconsistent for certain cadences. DR2 for example may be 359,297,259, sometimes 359,0,0, sometimes 297,0,0. The inconsistent DR patterns may or may not be related to the CID failure, although there appears to be a correlation. There may be leftover artifacts on CID detection between calls, as repeating a failed call tends to correlate to another failure. - Scott Griepentrog On Feb. 23, 2015, 6:51 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r// --- (Updated Feb. 23, 2015, 6:51 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24825 https://issues.asterisk.org/jira/browse/ASTERISK-24825 Repository: Asterisk Description --- The distinctive ring feature interferes with detecting Caller ID and appears to have been broken for years. What happens is if you have a ring-ring cadence as used in the UK you get too many DAHDI events for the distinctive ring pattern array and Caller ID detection is aborted. I think when Zapata/DAHDI added the ring begin event it broke distinctive ring. More events happen than before and the code does no filtering of which event times are recorded in the pattern array. * Made distinctive ring only record the ringt count when the ring ends instead of on just any DAHDI event. Distinctive ring can be ring, ring-ring, ring-ring-ring, or different ring durations for the up to three rings. * Fixed the distinctive ring detection enable (chan_dahdi.conf option usedistinctiveringdetection) to be per port instead of somewhat per port and somewhat global. This has been broken since v1.8. * Fixed using the default distinctive ring context when the detected pattern does not match any configured dringX patterns. The default context did not get set when the previous call was a matched distinctive ring pattern and the current call is not matched. This has been broken since v1.8. * Made distinctive ring have no effect on Caller ID detection when it is disabled. Caller ID detection just monitors for 10 seconds before giving up. * Fixed leak of struct callerid_state memory when a polarity reversal during Caller ID detection causes the incoming call to be aborted. Diffs - /branches/11/channels/sig_analog.c 432173 /branches/11/channels/sig_analog.h 432173 /branches/11/channels/chan_dahdi.c 432173 /branches/11/UPGRADE.txt 432173 Diff: https://reviewboard.asterisk.org/r//diff/ Testing --- Tested the following scenarios for US (ring) and UK (ring-ring) ring cadences: 1) usedistinctiveringdetection=no 2) usedistinctiveringdetection=yes with distinctiveringaftercid=no 3) usedistinctiveringdetection=yes with distinctiveringaftercid=yes * Caller-ID was detected for each call * The configured distinctive ring dringX pattern was detected and the specified context set. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4444: chan_dahdi/sig_analog: Fix distinctive ring detection to suck less.
On Feb. 27, 2015, 9:39 a.m., Scott Griepentrog wrote: I updated the chan_dahdi [channels] configuration with cadences matching the Bellcore spec and retested for both CID detection and DR pattern recognition. Results are certain patterns (especially DR2, DR4, and DR5) exhibit failure rate on CID detection approaching 50%. As the DR1 pattern has a 0% failure rate, I'm confident that it's still a remaining DR problem. Additionally, the DR pattern numbers (#,#,#) are inconsistent for certain cadences. DR2 for example may be 359,297,259, sometimes 359,0,0, sometimes 297,0,0. The inconsistent DR patterns may or may not be related to the CID failure, although there appears to be a correlation. There may be leftover artifacts on CID detection between calls, as repeating a failed call tends to correlate to another failure. I forgot to paste in my cadences: usedistinctiveringdetection=yes ;Bellcore-r1: ;60(2/4) cadence=2000,-4000 ;Bellcore-r2: ;60(.3/.2,1/.2,.3/4) cadence=300,200,1000,200,300,-4000 ;Bellcore-r3: ;60(.8/.4,.8/4) cadence=800,400,800,-4000 ;Bellcore-r4: ;60(.4/.2,.3/.2,.8/4) cadence=400,200,300,200,800,-4000 ;Bellcore-r5: ;60(.2/.2,.2/.2,.2/.2,1/4) cadence=200,200,200,200,200,200,1000,-4000 ;Bellcore-r6: ;60(.2/.4,.2/.4,.2/4) cadence=200,400,200,400,200,-4000 ;Bellcore-r7: ;60(.4/.2,.4/.2,.4/4) cadence=400,200,400,200,400,-4000 ;Bellcore-r8: ;60(0.25/9.75) cadence=250,-9750 - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r//#review14562 --- On Feb. 23, 2015, 6:51 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r// --- (Updated Feb. 23, 2015, 6:51 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24825 https://issues.asterisk.org/jira/browse/ASTERISK-24825 Repository: Asterisk Description --- The distinctive ring feature interferes with detecting Caller ID and appears to have been broken for years. What happens is if you have a ring-ring cadence as used in the UK you get too many DAHDI events for the distinctive ring pattern array and Caller ID detection is aborted. I think when Zapata/DAHDI added the ring begin event it broke distinctive ring. More events happen than before and the code does no filtering of which event times are recorded in the pattern array. * Made distinctive ring only record the ringt count when the ring ends instead of on just any DAHDI event. Distinctive ring can be ring, ring-ring, ring-ring-ring, or different ring durations for the up to three rings. * Fixed the distinctive ring detection enable (chan_dahdi.conf option usedistinctiveringdetection) to be per port instead of somewhat per port and somewhat global. This has been broken since v1.8. * Fixed using the default distinctive ring context when the detected pattern does not match any configured dringX patterns. The default context did not get set when the previous call was a matched distinctive ring pattern and the current call is not matched. This has been broken since v1.8. * Made distinctive ring have no effect on Caller ID detection when it is disabled. Caller ID detection just monitors for 10 seconds before giving up. * Fixed leak of struct callerid_state memory when a polarity reversal during Caller ID detection causes the incoming call to be aborted. Diffs - /branches/11/channels/sig_analog.c 432173 /branches/11/channels/sig_analog.h 432173 /branches/11/channels/chan_dahdi.c 432173 /branches/11/UPGRADE.txt 432173 Diff: https://reviewboard.asterisk.org/r//diff/ Testing --- Tested the following scenarios for US (ring) and UK (ring-ring) ring cadences: 1) usedistinctiveringdetection=no 2) usedistinctiveringdetection=yes with distinctiveringaftercid=no 3) usedistinctiveringdetection=yes with distinctiveringaftercid=yes * Caller-ID was detected for each call * The configured distinctive ring dringX pattern was detected and the specified context set. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4443: dial api: add self destruction option
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4443/ --- (Updated Feb. 26, 2015, 12:53 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 432385 Repository: Asterisk Description --- This adds a self-destruction ability to the dial api. The usefulness of this is mostly when using async mode to spawn a separate thread to handle the new call, while the calling thread is allowed to go on about other business. The alternative of this option is that the calling thread must either hang around for the duration or spawn it's own thread in order to create and then later destroy the dial structure after the call completes. Example (minus error checking): struct ast_dial *dial = ast_dial_create(); ast_dial_append(dial, PJSIP, 200, NULL); ast_dial_option_global_enable(dial, AST_DIAL_OPTION_ANSWER_EXEC, Echo); ast_dial_option_global_enable(dial, AST_DIAL_OPTION_SELF_DESTROY, NULL); ast_dial_run(dial, NULL, 1); The dial_run call returns almost immediately after spawning a new thread to complete and monitor the dial. If the call is answered, it is put into echo. When completed, ast_dial_destroy() will be called on the dial structure. Note that any allocations made to pass values to ast_dial_set_user_data() or other dial options will need to be free'd in a state callback function on any of AST_DIAL_RESULT_UNASWERED, AST_DIAL_RESULT_ANSWERED, AST_DIAL_RESULT_HANGUP, or AST_DIAL_RESULT_TIMEOUT. Diffs - /branches/13/main/dial.c 432173 /branches/13/include/asterisk/dial.h 432173 Diff: https://reviewboard.asterisk.org/r/4443/diff/ Testing --- Correct operation confirmed with a temporary test function running under valgrind to insure there are no invalid references or leaks. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4444: chan_dahdi/sig_analog: Fix distinctive ring detection to suck less.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r//#review14533 --- Tested with a looped tdm board generating various DR patterns and confirmed to resolve CID detection problem. Ran out of time to validate DR detection, will attempt later. - Scott Griepentrog On Feb. 23, 2015, 6:51 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r// --- (Updated Feb. 23, 2015, 6:51 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24825 https://issues.asterisk.org/jira/browse/ASTERISK-24825 Repository: Asterisk Description --- The distinctive ring feature interferes with detecting Caller ID and appears to have been broken for years. What happens is if you have a ring-ring cadence as used in the UK you get too many DAHDI events for the distinctive ring pattern array and Caller ID detection is aborted. I think when Zapata/DAHDI added the ring begin event it broke distinctive ring. More events happen than before and the code does no filtering of which event times are recorded in the pattern array. * Made distinctive ring only record the ringt count when the ring ends instead of on just any DAHDI event. Distinctive ring can be ring, ring-ring, ring-ring-ring, or different ring durations for the up to three rings. * Fixed the distinctive ring detection enable (chan_dahdi.conf option usedistinctiveringdetection) to be per port instead of somewhat per port and somewhat global. This has been broken since v1.8. * Fixed using the default distinctive ring context when the detected pattern does not match any configured dringX patterns. The default context did not get set when the previous call was a matched distinctive ring pattern and the current call is not matched. This has been broken since v1.8. * Made distinctive ring have no effect on Caller ID detection when it is disabled. Caller ID detection just monitors for 10 seconds before giving up. * Fixed leak of struct callerid_state memory when a polarity reversal during Caller ID detection causes the incoming call to be aborted. Diffs - /branches/11/channels/sig_analog.c 432173 /branches/11/channels/sig_analog.h 432173 /branches/11/channels/chan_dahdi.c 432173 /branches/11/UPGRADE.txt 432173 Diff: https://reviewboard.asterisk.org/r//diff/ Testing --- Tested the following scenarios for US (ring) and UK (ring-ring) ring cadences: 1) usedistinctiveringdetection=no 2) usedistinctiveringdetection=yes with distinctiveringaftercid=no 3) usedistinctiveringdetection=yes with distinctiveringaftercid=yes * Caller-ID was detected for each call * The configured distinctive ring dringX pattern was detected and the specified context set. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4407: cleanup various issues discovered during leak testing
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4407/ --- (Updated Feb. 6, 2015, 3:26 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 431582 Repository: Asterisk Description --- A collection of small fixes to prevent Valgrind errors for your amusement and approval: 1) Config hooks: a) don't hold extra reference after creation of a hook, b) cleanup the container on shutdown 2) Utils.c: ast_join_delim: don't touch w[x] if x out of bounds of size 3) res_pjsip sync_task: prevent reference to sync task data struct if it gets freed after unlocked. Diffs - /branches/13/res/res_pjsip.c 431571 /branches/13/main/utils.c 431571 /branches/13/main/config.c 431571 Diff: https://reviewboard.asterisk.org/r/4407/diff/ Testing --- In each case Valgrind did not show the same error again after the patch was added. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4141: Enable REF_DEBUG for ast_module_ref / ast_module_unref
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4141/#review14426 --- Ship it! Ship It! /branches/11/include/asterisk/module.h https://reviewboard.asterisk.org/r/4141/#comment24954 red blob - Scott Griepentrog On Feb. 5, 2015, 2:36 a.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4141/ --- (Updated Feb. 5, 2015, 2:36 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24479 https://issues.asterisk.org/jira/browse/ASTERISK-24479 Repository: Asterisk Description --- This change includes an ABI change with compatibility stubs for 11 and 13. The compatibility stubs will not be included in trunk. The point of this change is to have each module create an AO2 object on load, and hopefully destroy it on unload. This allows module reference count errors to be debugged through REF_DEBUG. When REF_DEBUG is enabled: * adds an empty ao2 object to 'struct ast_module' * Allocate ao2 when the module is loaded * Perform an ao2_ref in each place where mod-usecount is manipulated. * ao2_cleanup on module unload. The passthrough of file, line and func is needed for the REF_DEBUG to be of any use, so without the ABI changes this is not useful. The change to bridge_builtin_features.c ensures that the module cannot be manually unloaded, but is able to be unloaded during ast_module_shutdown. Note ast_module_shutdown only happens during clean shutdown and does not actually run dlclose so this is safe. Diffs - /branches/11/main/loader.c 429406 /branches/11/include/asterisk/module.h 429406 /branches/11/bridges/bridge_builtin_features.c 429406 Diff: https://reviewboard.asterisk.org/r/4141/diff/ Testing --- Using tests/manager/originate with REF_DEBUG enabled. When the change to bridge_builtin_features.c is omitted the test fails due to that one reference leak. Thanks, Corey Farrell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4407: cleanup various issues discovered during leak testing
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4407/ --- (Updated Feb. 6, 2015, 7:37 a.m.) Review request for Asterisk Developers. Changes --- Added null as Richard suggested. Repository: Asterisk Description (updated) --- A collection of small fixes to prevent Valgrind errors for your amusement and approval: 1) Config hooks: a) don't hold extra reference after creation of a hook, b) cleanup the container on shutdown 2) Utils.c: ast_join_delim: don't touch w[x] if x out of bounds of size 3) res_pjsip sync_task: prevent reference to sync task data struct if it gets freed after unlocked. Diffs (updated) - /branches/13/res/res_pjsip.c 431571 /branches/13/main/utils.c 431571 /branches/13/main/config.c 431571 Diff: https://reviewboard.asterisk.org/r/4407/diff/ Testing --- In each case Valgrind did not show the same error again after the patch was added. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 4407: cleanup various issues discovered during leak testing
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4407/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- A collection of small fixes to prevent Valgrind errors for your amusement and approval: 1) Config hooks: a) don't hold extra reference after creation of a hook, b) cleanup the container on shutdown 2) Util ast_join_delim: don't touch w[x] if x out of bounds of size 3) res_pjsip sync_task: prevent reference to sync task data struct if it gets freed after unlocked. Diffs - /branches/13/res/res_pjsip.c 431571 /branches/13/main/utils.c 431571 /branches/13/main/config.c 431571 Diff: https://reviewboard.asterisk.org/r/4407/diff/ Testing --- In each case Valgrind did not show the same error again after the patch was added. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4383: res_pjsip_config_wizard: Add ability to auto-create hints.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4383/#review14394
---
The dialplan Dial() app entry can conflict with existing dialplan in the same
context. I'm assuming that I won't enable the hint dialplan unless I mean to
do it through the wizard, but there is no option to skip the addition of the
Dial() app also. I would recommend either a) dropping the Dial() app but keep
the hint, b) requiring the app be specified in the config or it won't be added.
In either case, you need to clearly document the interaction between
pjsip_wizard added dialplan and extensions.conf entries - what will asterisk do
if it's put in both, and will reloading either module change which dialplan is
executed? Make sure to cover both the explicit extension conflict cases i.e.
200 vs 200 and the wildcard _2XX vs 200.
- Scott Griepentrog
On Jan. 27, 2015, 8:23 p.m., George Joseph wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4383/
---
(Updated Jan. 27, 2015, 8:23 p.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
Looking at the Super Awesome Company sample reminded me that creating hints
is just plain gruntwork. So you can now have the pjsip conifg wizard
auto-create them for you.
Specifying 'hint_exten' in the wizard will create 'exten =
hint_exten,hint/PJSIP/wizard_id' in whatever is specified for
'hint_context'.
Specifying 'hint_application' in the wizard will create 'exten =
hint_exten,1,hint_application' in whatever is specified for
'hint_context'.
The default for 'hint_context' is the endpoint's context.
The default for 'hint_application' is 'Dial(${HINT})'.
There's no default for 'hint_exten'. If not specified, neither the hint
itself nor the application will be created.
Some may think this is the slippery slope to users.conf but hints are a basic
necessity for phones unlike voicemail, manager, etc that users.conf creates.
Diffs
-
branches/13/res/res_pjsip_config_wizard.c 431242
branches/13/configs/samples/pjsip_wizard.conf.sample 431242
Diff: https://reviewboard.asterisk.org/r/4383/diff/
Testing
---
Existing config_wizard testsuite tests pass.
Additional testsuite tests in the works.
Thanks,
George Joseph
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4141: Enable REF_DEBUG for ast_module_ref / ast_module_unref
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4141/#review14395 --- /branches/11/main/loader.c https://reviewboard.asterisk.org/r/4141/#comment24892 I would recommend using a variable name such as ref_debug to better describe it's purpose. - Scott Griepentrog On Nov. 2, 2014, 1:13 a.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4141/ --- (Updated Nov. 2, 2014, 1:13 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24479 https://issues.asterisk.org/jira/browse/ASTERISK-24479 Repository: Asterisk Description --- This change includes an ABI change with compatibility stubs for 11, 12 and 13. The compatibility stubs will not be included in trunk. The point of this change is to have each module create an AO2 object on load, and hopefully destroy it on unload. This allows module reference count errors to be debugged through REF_DEBUG. When REF_DEBUG is enabled: * adds an empty ao2 object to 'struct ast_module' * Allocate ao2 when the module is loaded * Perform an ao2_ref in each place where mod-usecount is manipulated. * ao2_cleanup on module unload. The passthrough of file, line and func is needed for the REF_DEBUG to be of any use, so without the ABI changes this is not useful. The change to bridge_builtin_features.c ensures that the module cannot be manually unloaded, but is able to be unloaded during ast_module_shutdown. Note ast_module_shutdown only happens during clean shutdown and does not actually run dlclose so this is safe. Diffs - /branches/11/main/loader.c 426830 /branches/11/include/asterisk/module.h 426830 /branches/11/bridges/bridge_builtin_features.c 426830 Diff: https://reviewboard.asterisk.org/r/4141/diff/ Testing --- Using tests/manager/originate with REF_DEBUG enabled. When the change to bridge_builtin_features.c is omitted the test fails due to that one reference leak. Thanks, Corey Farrell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4382: stasis bridge: handle early hangup of swap channel
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4382/ --- (Updated Jan. 29, 2015, 11:20 a.m.) Review request for Asterisk Developers. Changes --- Addressed Richard's findings. Bugs: ASTERISK-24649 https://issues.asterisk.org/jira/browse/ASTERISK-24649 Repository: Asterisk Description --- During an attended transfer of a channel in stasis, where a local channel is used to replace (swap in for) a PJSIP channel, there is a race condition. The PJSIP REFER transfer has been initiated (ast_bridge_impart) but will be completed later on another thread (bridge_channel_ind_thread), however, prior to the stasis bridging callback (bridge_stasis_push) being called the PJSIP channel is hungup, which allows the stasis app loop to exit, deleting the stasis control. That prevents the stasis bridge callback from getting the app name from the channel being replaced (swap). This patch adds a new stasis bridge callback push_peek, which is called from the ast_bridge_impart() thread. This allows the new bridge_stasis_push_peek() function to copy the stasis app name from the originating channel before the PJSIP channel can be hungup. Diffs (updated) - /branches/13/res/stasis/stasis_bridge.c 431296 /branches/13/main/bridge.c 431296 /branches/13/include/asterisk/bridge.h 431296 Diff: https://reviewboard.asterisk.org/r/4382/diff/ Testing --- Running tests/rest_api/external_interaction/attended_transfer/stasis_bridge_to_stasis_app now does not result in an occasional failure. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4382: stasis bridge: handle early hangup of swap channel
On Jan. 28, 2015, 3:08 p.m., rmudgett wrote: /branches/13/include/asterisk/bridge.h, line 245 https://reviewboard.asterisk.org/r/4382/diff/4/?file=71156#file71156line245 push_peek should not use push's typedef. It should use its own in case they need to diverge since they are called for slightly different purposes. I disagree -- they shouldn't diverge since the purpose of push_peek() is to be an early version of push() run from the original thread, and run with the same exact arguments and return values. - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4382/#review14347 --- On Jan. 29, 2015, 11:20 a.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4382/ --- (Updated Jan. 29, 2015, 11:20 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24649 https://issues.asterisk.org/jira/browse/ASTERISK-24649 Repository: Asterisk Description --- During an attended transfer of a channel in stasis, where a local channel is used to replace (swap in for) a PJSIP channel, there is a race condition. The PJSIP REFER transfer has been initiated (ast_bridge_impart) but will be completed later on another thread (bridge_channel_ind_thread), however, prior to the stasis bridging callback (bridge_stasis_push) being called the PJSIP channel is hungup, which allows the stasis app loop to exit, deleting the stasis control. That prevents the stasis bridge callback from getting the app name from the channel being replaced (swap). This patch adds a new stasis bridge callback push_peek, which is called from the ast_bridge_impart() thread. This allows the new bridge_stasis_push_peek() function to copy the stasis app name from the originating channel before the PJSIP channel can be hungup. Diffs - /branches/13/res/stasis/stasis_bridge.c 431296 /branches/13/main/bridge.c 431296 /branches/13/include/asterisk/bridge.h 431296 Diff: https://reviewboard.asterisk.org/r/4382/diff/ Testing --- Running tests/rest_api/external_interaction/attended_transfer/stasis_bridge_to_stasis_app now does not result in an occasional failure. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4382: stasis bridge: handle early hangup of swap channel
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4382/ --- (Updated Jan. 29, 2015, 5:02 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 431450 Bugs: ASTERISK-24649 https://issues.asterisk.org/jira/browse/ASTERISK-24649 Repository: Asterisk Description --- During an attended transfer of a channel in stasis, where a local channel is used to replace (swap in for) a PJSIP channel, there is a race condition. The PJSIP REFER transfer has been initiated (ast_bridge_impart) but will be completed later on another thread (bridge_channel_ind_thread), however, prior to the stasis bridging callback (bridge_stasis_push) being called the PJSIP channel is hungup, which allows the stasis app loop to exit, deleting the stasis control. That prevents the stasis bridge callback from getting the app name from the channel being replaced (swap). This patch adds a new stasis bridge callback push_peek, which is called from the ast_bridge_impart() thread. This allows the new bridge_stasis_push_peek() function to copy the stasis app name from the originating channel before the PJSIP channel can be hungup. Diffs - /branches/13/res/stasis/stasis_bridge.c 431296 /branches/13/main/bridge.c 431296 /branches/13/include/asterisk/bridge.h 431296 Diff: https://reviewboard.asterisk.org/r/4382/diff/ Testing --- Running tests/rest_api/external_interaction/attended_transfer/stasis_bridge_to_stasis_app now does not result in an occasional failure. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4382: stasis bridge: handle early hangup of swap channel
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4382/ --- (Updated Jan. 28, 2015, 11:01 a.m.) Review request for Asterisk Developers. Changes --- Addressed Matt's and Richard's findings. Completed implementation of push_peek() call to match push(). Bugs: ASTERISK-24649 https://issues.asterisk.org/jira/browse/ASTERISK-24649 Repository: Asterisk Description --- During an attended transfer of a channel in stasis, where a local channel is used to replace (swap in for) a PJSIP channel, there is a race condition. The PJSIP REFER transfer has been initiated (ast_bridge_impart) but will be completed later on another thread (bridge_channel_ind_thread), however, prior to the stasis bridging callback (bridge_stasis_push) being called the PJSIP channel is hungup, which allows the stasis app loop to exit, deleting the stasis control. That prevents the stasis bridge callback from getting the app name from the channel being replaced (swap). This patch adds a new stasis bridge callback push_peek, which is called from the ast_bridge_impart() thread. This allows the new bridge_stasis_push_peek() function to copy the stasis app name from the originating channel before the PJSIP channel can be hungup. Diffs (updated) - /branches/13/res/stasis/stasis_bridge.c 431296 /branches/13/main/bridge_channel.c 431296 /branches/13/main/bridge.c 431296 /branches/13/include/asterisk/bridge.h 431296 Diff: https://reviewboard.asterisk.org/r/4382/diff/ Testing --- Running tests/rest_api/external_interaction/attended_transfer/stasis_bridge_to_stasis_app now does not result in an occasional failure. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4382: stasis bridge: handle early hangup of swap channel
On Jan. 28, 2015, 10:42 a.m., rmudgett wrote: /branches/13/include/asterisk/bridge.h, lines 244-245 https://reviewboard.asterisk.org/r/4382/diff/2/?file=71145#file71145line244 Create a new typedef for the new callback. Also the new callback doesn't need the swap parameter since that pointer is passed in with the bridge_channel parameter struct. I'm assuming that there could be a case where having the swap's bridge_channel structure could be needed for other/future implementations. I didn't write the support in for obtaining and passing that because I didn't need it to solve this problem. Additionally, the return code should actually be checked to allow peek to cause a transfer to fail. I'll go ahead and implement both. - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4382/#review14341 --- On Jan. 27, 2015, 8:55 p.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4382/ --- (Updated Jan. 27, 2015, 8:55 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24649 https://issues.asterisk.org/jira/browse/ASTERISK-24649 Repository: Asterisk Description --- During an attended transfer of a channel in stasis, where a local channel is used to replace (swap in for) a PJSIP channel, there is a race condition. The PJSIP REFER transfer has been initiated (ast_bridge_impart) but will be completed later on another thread (bridge_channel_ind_thread), however, prior to the stasis bridging callback (bridge_stasis_push) being called the PJSIP channel is hungup, which allows the stasis app loop to exit, deleting the stasis control. That prevents the stasis bridge callback from getting the app name from the channel being replaced (swap). This patch adds a new stasis bridge callback push_peek, which is called from the ast_bridge_impart() thread. This allows the new bridge_stasis_push_peek() function to copy the stasis app name from the originating channel before the PJSIP channel can be hungup. Diffs - /branches/13/res/stasis/stasis_bridge.c 431242 /branches/13/main/bridge_channel.c 431242 /branches/13/main/bridge.c 431242 /branches/13/include/asterisk/bridge.h 431242 Diff: https://reviewboard.asterisk.org/r/4382/diff/ Testing --- Running tests/rest_api/external_interaction/attended_transfer/stasis_bridge_to_stasis_app now does not result in an occasional failure. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4382: stasis bridge: handle early hangup of swap channel
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4382/ --- (Updated Jan. 28, 2015, 1:35 p.m.) Review request for Asterisk Developers. Changes --- Addressed Richard's findings. Also added matching call to push_peek on ast_bridge_join after discussion. Left the other uses of swap in push() alone since they shouldn't be affected by lack of swap on early hangup of replaced channel. Bugs: ASTERISK-24649 https://issues.asterisk.org/jira/browse/ASTERISK-24649 Repository: Asterisk Description --- During an attended transfer of a channel in stasis, where a local channel is used to replace (swap in for) a PJSIP channel, there is a race condition. The PJSIP REFER transfer has been initiated (ast_bridge_impart) but will be completed later on another thread (bridge_channel_ind_thread), however, prior to the stasis bridging callback (bridge_stasis_push) being called the PJSIP channel is hungup, which allows the stasis app loop to exit, deleting the stasis control. That prevents the stasis bridge callback from getting the app name from the channel being replaced (swap). This patch adds a new stasis bridge callback push_peek, which is called from the ast_bridge_impart() thread. This allows the new bridge_stasis_push_peek() function to copy the stasis app name from the originating channel before the PJSIP channel can be hungup. Diffs (updated) - /branches/13/res/stasis/stasis_bridge.c 431296 /branches/13/main/bridge.c 431296 /branches/13/include/asterisk/bridge.h 431296 Diff: https://reviewboard.asterisk.org/r/4382/diff/ Testing --- Running tests/rest_api/external_interaction/attended_transfer/stasis_bridge_to_stasis_app now does not result in an occasional failure. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 4382: stasis bridge: handle early hangup up swap channel
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4382/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24649 https://issues.asterisk.org/jira/browse/ASTERISK-24649 Repository: Asterisk Description --- During an attended transfer of a channel in stasis, where a local channel is used to replace (swap in for) a PJSIP channel, there is a race condition. The PJSIP REFER transfer has been initiated (ast_bridge_impart) but will be completed later on another thread (bridge_channel_ind_thread), however, prior to the stasis bridging callback (bridge_stasis_push) being called the PJSIP channel is hungup, which allows the stasis app loop to exit, deleting the stasis control. That prevents the stasis bridge callback from getting the app name from the channel being replaced (swap). This patch adds a new stasis bridge callback push_peek, which is called from the ast_bridge_impart() thread. This allows the new bridge_stasis_push_peek() function to copy the stasis app name from the originating channel before the PJSIP channel can be hungup. Diffs - /branches/13/res/stasis/stasis_bridge.c 431217 /branches/13/main/bridge_channel.c 431217 /branches/13/main/bridge.c 431217 /branches/13/include/asterisk/bridge.h 431217 Diff: https://reviewboard.asterisk.org/r/4382/diff/ Testing --- Running tests/rest_api/external_interaction/attended_transfer/stasis_bridge_to_stasis_app now does not result in an occasional failure. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4382: stasis bridge: handle early hangup of swap channel
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4382/ --- (Updated Jan. 27, 2015, 8:11 p.m.) Review request for Asterisk Developers. Changes --- english enhancement in title Summary (updated) - stasis bridge: handle early hangup of swap channel Bugs: ASTERISK-24649 https://issues.asterisk.org/jira/browse/ASTERISK-24649 Repository: Asterisk Description --- During an attended transfer of a channel in stasis, where a local channel is used to replace (swap in for) a PJSIP channel, there is a race condition. The PJSIP REFER transfer has been initiated (ast_bridge_impart) but will be completed later on another thread (bridge_channel_ind_thread), however, prior to the stasis bridging callback (bridge_stasis_push) being called the PJSIP channel is hungup, which allows the stasis app loop to exit, deleting the stasis control. That prevents the stasis bridge callback from getting the app name from the channel being replaced (swap). This patch adds a new stasis bridge callback push_peek, which is called from the ast_bridge_impart() thread. This allows the new bridge_stasis_push_peek() function to copy the stasis app name from the originating channel before the PJSIP channel can be hungup. Diffs - /branches/13/res/stasis/stasis_bridge.c 431217 /branches/13/main/bridge_channel.c 431217 /branches/13/main/bridge.c 431217 /branches/13/include/asterisk/bridge.h 431217 Diff: https://reviewboard.asterisk.org/r/4382/diff/ Testing --- Running tests/rest_api/external_interaction/attended_transfer/stasis_bridge_to_stasis_app now does not result in an occasional failure. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4382: stasis bridge: handle early hangup of swap channel
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4382/ --- (Updated Jan. 27, 2015, 8:55 p.m.) Review request for Asterisk Developers. Changes --- Addressed Richard's issues Bugs: ASTERISK-24649 https://issues.asterisk.org/jira/browse/ASTERISK-24649 Repository: Asterisk Description --- During an attended transfer of a channel in stasis, where a local channel is used to replace (swap in for) a PJSIP channel, there is a race condition. The PJSIP REFER transfer has been initiated (ast_bridge_impart) but will be completed later on another thread (bridge_channel_ind_thread), however, prior to the stasis bridging callback (bridge_stasis_push) being called the PJSIP channel is hungup, which allows the stasis app loop to exit, deleting the stasis control. That prevents the stasis bridge callback from getting the app name from the channel being replaced (swap). This patch adds a new stasis bridge callback push_peek, which is called from the ast_bridge_impart() thread. This allows the new bridge_stasis_push_peek() function to copy the stasis app name from the originating channel before the PJSIP channel can be hungup. Diffs (updated) - /branches/13/res/stasis/stasis_bridge.c 431242 /branches/13/main/bridge_channel.c 431242 /branches/13/main/bridge.c 431242 /branches/13/include/asterisk/bridge.h 431242 Diff: https://reviewboard.asterisk.org/r/4382/diff/ Testing --- Running tests/rest_api/external_interaction/attended_transfer/stasis_bridge_to_stasis_app now does not result in an occasional failure. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4372: confbridge: Restore menu name associated with confbridge user to CLI output of 'confbridge list XXX'
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4372/#review14290 --- /branches/13/apps/confbridge/conf_config_parser.c https://reviewboard.asterisk.org/r/4372/#comment24772 Use ast_copy_string? - Scott Griepentrog On Jan. 26, 2015, 10:28 a.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4372/ --- (Updated Jan. 26, 2015, 10:28 a.m.) Review request for Asterisk Developers and Jonathan Rose. Bugs: ASTERISK-24723 https://issues.asterisk.org/jira/browse/ASTERISK-24723 Repository: Asterisk Description --- When issuing a 'confbridge list ' CLI command, the resulting output no longer displays the menu associated with a ConfBridge participant: ChannelFlags User Profile Bridge Profile Menu CallerID == == PJSIP/1601-0004 default_user default_bridge 1601 The issue was caused by ASTERISK-22760. When that patch was done, it removed the copying of the menu name associated with the user from the actual user profile. This patch fixes the issue by copying the menu name over to the user profile when the menu hooks are applied to the user. Since that function now does a little bit more than just apply the hooks, the name of the function has been changed to cover the copying of the menu name over as well. In addition, there is a disparity between the menu name length as it is stored on the conf_menu structure and the confbridge_user structure; this patch makes the lengths match so that a strcpy can be used. Diffs - /branches/13/apps/confbridge/include/confbridge.h 431019 /branches/13/apps/confbridge/conf_config_parser.c 431019 Diff: https://reviewboard.asterisk.org/r/4372/diff/ Testing --- Ran the CLI command. The output now shows the menu associated with the user. Thanks, Matt Jordan -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4341: stasis transfers: fix race condition on set replace channel
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4341/ --- (Updated Jan. 22, 2015, 12:09 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 430939 Bugs: ASTERISK-24649 https://issues.asterisk.org/jira/browse/ASTERISK-24649 Repository: Asterisk Description --- After a transfer completes that uses a local replacement channel, stasis receives the stasis transfer message with the details of the transfer and makes changes on the replacement channel. However, since a separate thread was already started for the purpose of starting stasis on the new replacement channel, this allowed for a race condition. Occasionally later then normal arrival of the stasis transfer message would result in the stasis app name not being set on the replacement channel before it was needed by the other thread, causing it to fail to start stasis and hang up. This change moves the operations (assignment of the stasis app name and setting the replacement snapshot on the new channel) into the bridge_stasis_push() callback from the bridge transfer logic. This allows these steps to be completed earlier and more deterministically, eliminating the race condition. Diffs - /branches/13/res/stasis/stasis_bridge.c 430394 /branches/13/res/stasis/app.c 430394 Diff: https://reviewboard.asterisk.org/r/4341/diff/ Testing --- The stasis start/end tests that discovered the issue are now passing, and I've not found any other test failures. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4341: stasis transfers: fix race condition on set replace channel
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4341/ --- (Updated Jan. 20, 2015, 6:07 p.m.) Review request for Asterisk Developers. Changes --- Added removal of swap from bridge after discussion with Richard Bugs: ASTERISK-24649 https://issues.asterisk.org/jira/browse/ASTERISK-24649 Repository: Asterisk Description --- After a transfer completes that uses a local replacement channel, stasis receives the stasis transfer message with the details of the transfer and makes changes on the replacement channel. However, since a separate thread was already started for the purpose of starting stasis on the new replacement channel, this allowed for a race condition. Occasionally later then normal arrival of the stasis transfer message would result in the stasis app name not being set on the replacement channel before it was needed by the other thread, causing it to fail to start stasis and hang up. This change moves the operations (assignment of the stasis app name and setting the replacement snapshot on the new channel) into the bridge_stasis_push() callback from the bridge transfer logic. This allows these steps to be completed earlier and more deterministically, eliminating the race condition. Diffs (updated) - /branches/13/res/stasis/stasis_bridge.c 430394 /branches/13/res/stasis/app.c 430394 Diff: https://reviewboard.asterisk.org/r/4341/diff/ Testing --- The stasis start/end tests that discovered the issue are now passing, and I've not found any other test failures. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4341: stasis transfers: fix race condition on set replace channel
On Jan. 16, 2015, 6:25 p.m., rmudgett wrote: /branches/13/res/stasis/stasis_bridge.c, lines 136-137 https://reviewboard.asterisk.org/r/4341/diff/2/?file=70635#file70635line136 May need to add this call here to ensure that the swap channel leaves the bridge: ast_bridge_channel_leave_bridge(swap, BRIDGE_CHANNEL_STATE_END_NO_DISSOLVE, 0); Have to test to see if it is needed. From manual testing with PJSIP endpoints, there is no apparent difference in operation with or without the suggested call. - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4341/#review14214 --- On Jan. 16, 2015, 3:58 p.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4341/ --- (Updated Jan. 16, 2015, 3:58 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24649 https://issues.asterisk.org/jira/browse/ASTERISK-24649 Repository: Asterisk Description --- After a transfer completes that uses a local replacement channel, stasis receives the stasis transfer message with the details of the transfer and makes changes on the replacement channel. However, since a separate thread was already started for the purpose of starting stasis on the new replacement channel, this allowed for a race condition. Occasionally later then normal arrival of the stasis transfer message would result in the stasis app name not being set on the replacement channel before it was needed by the other thread, causing it to fail to start stasis and hang up. This change moves the operations (assignment of the stasis app name and setting the replacement snapshot on the new channel) into the bridge_stasis_push() callback from the bridge transfer logic. This allows these steps to be completed earlier and more deterministically, eliminating the race condition. Diffs - /branches/13/res/stasis/stasis_bridge.c 430394 /branches/13/res/stasis/app.c 430394 Diff: https://reviewboard.asterisk.org/r/4341/diff/ Testing --- The stasis start/end tests that discovered the issue are now passing, and I've not found any other test failures. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4341: stasis transfers: fix race condition on set replace channel
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4341/ --- (Updated Jan. 16, 2015, 3:58 p.m.) Review request for Asterisk Developers. Changes --- Added documentation of push failure case. Bugs: ASTERISK-24649 https://issues.asterisk.org/jira/browse/ASTERISK-24649 Repository: Asterisk Description --- After a transfer completes that uses a local replacement channel, stasis receives the stasis transfer message with the details of the transfer and makes changes on the replacement channel. However, since a separate thread was already started for the purpose of starting stasis on the new replacement channel, this allowed for a race condition. Occasionally later then normal arrival of the stasis transfer message would result in the stasis app name not being set on the replacement channel before it was needed by the other thread, causing it to fail to start stasis and hang up. This change moves the operations (assignment of the stasis app name and setting the replacement snapshot on the new channel) into the bridge_stasis_push() callback from the bridge transfer logic. This allows these steps to be completed earlier and more deterministically, eliminating the race condition. Diffs (updated) - /branches/13/res/stasis/stasis_bridge.c 430394 /branches/13/res/stasis/app.c 430394 Diff: https://reviewboard.asterisk.org/r/4341/diff/ Testing --- The stasis start/end tests that discovered the issue are now passing, and I've not found any other test failures. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 4341: stasis transfers: fix race condition on set replace channel
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4341/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24649 https://issues.asterisk.org/jira/browse/ASTERISK-24649 Repository: Asterisk Description --- After a transfer completes that uses a local replacement channel, stasis receives the stasis transfer message with the details of the transfer and makes changes on the replacement channel. However, since a separate thread was already started for the purpose of starting stasis on the new replacement channel, this allowed for a race condition. Occasionally later then normal arrival of the stasis transfer message would result in the stasis app name not being set on the replacement channel before it was needed by the other thread, causing it to fail to start stasis and hang up. This change moves the operations (assignment of the stasis app name and setting the replacement snapshot on the new channel) into the bridge_stasis_push() callback from the bridge transfer logic. This allows these steps to be completed earlier and more deterministically, eliminating the race condition. Diffs - /branches/13/res/stasis/stasis_bridge.c 430394 /branches/13/res/stasis/app.c 430394 Diff: https://reviewboard.asterisk.org/r/4341/diff/ Testing --- The stasis start/end tests that discovered the issue are now passing, and I've not found any other test failures. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4322: app_bridge: return to next dialplan priority
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4322/ --- (Updated Jan. 9, 2015, 3:45 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 430467 Bugs: ASTERISK-24637 https://issues.asterisk.org/jira/browse/ASTERISK-24637 Repository: Asterisk Description --- When app_bridge grabs a channel to put in a bridge, it should allow it to continue executing dialplan after the bridge ends. Although the current dialplan is stored as an after bridge goto on the channel, it was executing the same priority of dialplan again rather than going to the next priority. This change replaces the specific version of bridge_set_after_goto with bridge_set_after_go_on to allow the dialplan execution to naturally flow to the next priority. Diffs - /branches/13/main/features.c 430220 Diff: https://reviewboard.asterisk.org/r/4322/diff/ Testing --- Testsuite test that caught problem now passes. I also ran the bridge_baseline and other bridge tests to insure they passed. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4256: testsuite: check for channel leak on failed blonde transfer
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4256/ --- (Updated Jan. 9, 2015, 3:58 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 6227 Bugs: ASTERISK-24513 https://issues.asterisk.org/jira/browse/ASTERISK-24513 Repository: testsuite Description --- This test starts an attended transfer, converts to blonde mode by hanging up the transferer, and then fails the transfer by hanging up the transferee. Then after allowing the recall attempt to complete, checks to insure that there are not leaked channels. Improvements to channel_test_condition: count the actual channels listed in core show channels output to check for leaks. Also added unittest. Diffs - /asterisk/trunk/tests/bridge/tests.yaml 6149 /asterisk/trunk/tests/bridge/atxfer_fail_blonde/test-config.yaml PRE-CREATION /asterisk/trunk/tests/bridge/atxfer_fail_blonde/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/lib/python/asterisk/channel_test_condition.py 6149 Diff: https://reviewboard.asterisk.org/r/4256/diff/ Testing --- Currently fails while ASTERISK-24513 is not yet patched. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4280: sip_to_pjsip: improved ability to parse input without exception
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4280/ --- (Updated Jan. 9, 2015, 4:09 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 430469 Bugs: ASTERISK-24474 https://issues.asterisk.org/jira/browse/ASTERISK-24474 Repository: Asterisk Description --- General improvements to reliability of conversion utility: 1) track default section of input file to allow parsing an include file that doesn't specify a [section] 2) informatively handle case of assignment with no section 3) correctly handle getting sections from included files 4) assume default bind of 0.0.0.0 5) gracefully handle missing portions of registration string 6) Denote steps of operation and confirm top level conf files being read/written as a convenience Diffs - /branches/12/contrib/scripts/sip_to_pjsip/sip_to_pjsip.py 429613 /branches/12/contrib/scripts/sip_to_pjsip/astconfigparser.py 429613 Diff: https://reviewboard.asterisk.org/r/4280/diff/ Testing --- Ran on config files from various sources to insure no exceptions occurred. Perused output to confirm appearance of converted input values. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 4322: app_bridge: return to next dialplan priority
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4322/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24637 https://issues.asterisk.org/jira/browse/ASTERISK-24637 Repository: Asterisk Description --- When app_bridge grabs a channel to put in a bridge, it should allow it to continue executing dialplan after the bridge ends. Although the current dialplan is stored as an after bridge goto on the channel, it was executing the same priority of dialplan again rather than going to the next priority. This change replaces the specific version of bridge_set_after_goto with bridge_set_after_go_on to allow the dialplan execution to naturally flow to the next priority. Diffs - /branches/13/main/features.c 430220 Diff: https://reviewboard.asterisk.org/r/4322/diff/ Testing --- Testsuite test that caught problem now passes. I also ran the bridge_baseline and other bridge tests to insure they passed. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4256: testsuite: check for channel leak on failed blonde transfer
On Jan. 5, 2015, 5 p.m., Scott Griepentrog wrote: Is this missing a sip.conf? It's using the bridge test case which is built on sip and includes it's own default sip.conf. Thus, I don't have to provide one in this test, and it would require changing the bridge test case, or at least creating a new test case - including support directories - so that one could easily switch a test to pjsip without much other changes. Which is actually not a bad idea, but out of the scope of getting this test in. - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4256/#review14078 --- On Dec. 31, 2014, 12:01 p.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4256/ --- (Updated Dec. 31, 2014, 12:01 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24513 https://issues.asterisk.org/jira/browse/ASTERISK-24513 Repository: testsuite Description --- This test starts an attended transfer, converts to blonde mode by hanging up the transferer, and then fails the transfer by hanging up the transferee. Then after allowing the recall attempt to complete, checks to insure that there are not leaked channels. Improvements to channel_test_condition: count the actual channels listed in core show channels output to check for leaks. Also added unittest. Diffs - /asterisk/trunk/tests/bridge/tests.yaml 6149 /asterisk/trunk/tests/bridge/atxfer_fail_blonde/test-config.yaml PRE-CREATION /asterisk/trunk/tests/bridge/atxfer_fail_blonde/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/lib/python/asterisk/channel_test_condition.py 6149 Diff: https://reviewboard.asterisk.org/r/4256/diff/ Testing --- Currently fails while ASTERISK-24513 is not yet patched. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4302: bridge: channel ref leak after failed blond transfer
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4302/ --- (Updated Jan. 5, 2015, 4:49 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 430199 Bugs: ASTERISK-24513 https://issues.asterisk.org/jira/browse/ASTERISK-24513 Repository: Asterisk Description --- After a blond transfer to a destination that then hangs up, the Local;1 channel would suffer a reference leak. This was supposed to be fixed by review 4262, but that caused an extra unreference on nominal blond transfer (somehow missed that in testing). This correction to the original problem reverts the prior change which had incorrectly mucked with moving the referenced transfer_target channel to recall_target in blond_nonfinal_enter(), and instead resolves the issue by insuring that the reference in recall_target is not lost by releasing it on blond_final_exit() before it is later replaced by an answered recall dial. Diffs - /branches/13/main/bridge_basic.c 430163 Diff: https://reviewboard.asterisk.org/r/4302/diff/ Testing --- Both the blond_nominal and atxfer_fail_blond (r/4256) tests pass without error. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4311: res_pjsip: make it unloadable
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4311/#review14074 --- I have tested and confirmed no failures under valgrind with unload and graceful shutdown. - Scott Griepentrog On Jan. 2, 2015, 1:46 p.m., Kevin Harwell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4311/ --- (Updated Jan. 2, 2015, 1:46 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24485 https://issues.asterisk.org/jira/browse/ASTERISK-24485 Repository: Asterisk Description --- The res_pjsip module was previously unloadable. With this patch it can now be unloaded. This patch is based off the original patch on the issue by Corey Farrell with a few modifications. Removed a few changes not required to make the module unloadable and also fixed a bug that would cause asterisk to crash on unloading. Diffs - branches/13/res/res_pjsip/pjsip_outbound_auth.c 430178 branches/13/res/res_pjsip/pjsip_options.c 430178 branches/13/res/res_pjsip/pjsip_global_headers.c 430178 branches/13/res/res_pjsip/pjsip_distributor.c 430178 branches/13/res/res_pjsip/pjsip_configuration.c 430178 branches/13/res/res_pjsip/location.c 430178 branches/13/res/res_pjsip/include/res_pjsip_private.h 430178 branches/13/res/res_pjsip/config_transport.c 430178 branches/13/res/res_pjsip/config_auth.c 430178 branches/13/res/res_pjsip.c 430178 branches/13/main/stasis_message_router.c 430178 Diff: https://reviewboard.asterisk.org/r/4311/diff/ Testing --- Made it so res_pjsip was the only pjsip module loaded and then issued an unload and noted it unloaded successfully (also loaded/unloaded it several times from the CLI). Also when loaded and with REF_DEBUG enabled issued a core stop gracefully and made sure there were no ref leaks for the module. Also tested unloading with other dependent pjsip modules loaded and noted that the module would not unload (as it should since dependencies are currently loaded). And then shutdown asterisk and made sure it did not crash or anything. Started asterisk with nominal and off nominal module and pjsip configurations to make sure things behaved appropriately (no crashes and such) and then attempted to, or successfully unload the res_pjsip module. Also made sure Asterisk continued to shutdown without incident. Thanks, Kevin Harwell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4302: bridge: channel ref leak after failed blond transfer
On Dec. 31, 2014, 3:46 p.m., Matt Jordan wrote: /branches/13/main/bridge_basic.c, lines 2398-2410 https://reviewboard.asterisk.org/r/4302/diff/1/?file=70051#file70051line2398 I'm not sure we should be clearing the reference here. Clearly, we need to clear the reference to props-recall_target if the channel answers. In that case, however, that could be done on line 2341. If, however, the target doesn't answer, then we may still have to do something with the recall_target. That, of course, depends on the current state that the state machine is in - but here, it looks like we've blown away any chance for the state machine to use the original recall_target reference, regardless of the outcome of the dial operation. When the leak occurs, what state is the state machine in? Are there states in the state machine that would like to reference the recall_target channel? After reviewing the state machine operation more closely, I've determined that recall_target is used only in the BLOND_NONFINAL, RECALLING, and RETRANSFER states. In each state, the enter function presumes that recall_target is not being used, and is free to set it to the referenced target channel. The exit function is then responsible for insuring that the reference is released, but this is only required if the next state is another recall, retransfer, or wait-to-recall/transfer. Any transition to FAIL or RESUME ot other state will end up cleaning up the recall_target on props destruction. Thus I've reworked this change to fix the leaked recall_target channel on blond_nonfinal_exit, to more properly match the state machine convention. - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4302/#review14062 --- On Jan. 2, 2015, 10:46 a.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4302/ --- (Updated Jan. 2, 2015, 10:46 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24513 https://issues.asterisk.org/jira/browse/ASTERISK-24513 Repository: Asterisk Description --- After a blond transfer to a destination that then hangs up, the Local;1 channel would suffer a reference leak. This was supposed to be fixed by review 4262, but that caused an extra unreference on nominal blond transfer (somehow missed that in testing). This correction to the original problem reverts the prior change which had incorrectly mucked with moving the referenced transfer_target channel to recall_target in blond_nonfinal_enter(), and instead resolves the issue by insuring that the reference in recall_target is not lost by releasing it on blond_final_exit() before it is later replaced by an answered recall dial. Diffs - /branches/13/main/bridge_basic.c 430163 Diff: https://reviewboard.asterisk.org/r/4302/diff/ Testing --- Both the blond_nominal and atxfer_fail_blond (r/4256) tests pass without error. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4302: bridge: channel ref leak after failed blond transfer
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4302/ --- (Updated Jan. 2, 2015, 10:46 a.m.) Review request for Asterisk Developers. Changes --- Moved unref of recall_target to state exit function to match existing state machine usage of the variable. Documented variable usage. Also found two cases of bridge_impart that would leak reference on failure. Bugs: ASTERISK-24513 https://issues.asterisk.org/jira/browse/ASTERISK-24513 Repository: Asterisk Description (updated) --- After a blond transfer to a destination that then hangs up, the Local;1 channel would suffer a reference leak. This was supposed to be fixed by review 4262, but that caused an extra unreference on nominal blond transfer (somehow missed that in testing). This correction to the original problem reverts the prior change which had incorrectly mucked with moving the referenced transfer_target channel to recall_target in blond_nonfinal_enter(), and instead resolves the issue by insuring that the reference in recall_target is not lost by releasing it on blond_final_exit() before it is later replaced by an answered recall dial. Diffs (updated) - /branches/13/main/bridge_basic.c 430163 Diff: https://reviewboard.asterisk.org/r/4302/diff/ Testing --- Both the blond_nominal and atxfer_fail_blond (r/4256) tests pass without error. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4286: rtp_engine: avoid payload types above 127
On Dec. 23, 2014, 7:41 a.m., Joshua Colp wrote: /trunk/main/rtp_engine.c, line 2193 https://reviewboard.asterisk.org/r/4286/diff/1/?file=69966#file69966line2193 Yeah, put this in the 96-127 rang instead. Wouldn't that cause ast_rtp_engine_load_format() to fail because no dynamic rtp mapping is available? - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4286/#review14028 --- On Dec. 19, 2014, 2:24 p.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4286/ --- (Updated Dec. 19, 2014, 2:24 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24367 https://issues.asterisk.org/jira/browse/ASTERISK-24367 Repository: Asterisk Description --- Valid payload type codes are between 0 and 127 to allow for being stored in 7 bits. During call setup, pjsip validates the SDP and will assert if it encounters an invalid payload type code (see pjmedia_sdp_validate() in pjmedia/src/pjmedia/sdp.c). This assert will be hit if a call is placed to a pjsip endpoint with allow=all set. To avoid this, the previous use 128 for the slin192 format has been changed to 95. Diffs - /trunk/main/rtp_engine.c 429845 Diff: https://reviewboard.asterisk.org/r/4286/diff/ Testing --- Tested with pjsip calls to allow=all configured extensions. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4280: sip_to_pjsip: improved ability to parse input without exception
On Dec. 23, 2014, 9:18 a.m., Matt Jordan wrote: /branches/12/contrib/scripts/sip_to_pjsip/astconfigparser.py, lines 334-335 https://reviewboard.asterisk.org/r/4280/diff/2/?file=69851#file69851line334 This could be written as: res.extend([inc.get_sections(key, attr, searched) for inc in self._includes]) Two reasons for this change: (1) generally, list comprehensions are a bit faster than for loops that build a list. (2) using an explicit extend with a list comprehension informs the reader of the intent of the code over the += operator. (Note: the difference between '+' and '+=' on a list caught me by surprise the first time. I'm less a fan of using operators on a list than using explicit methods.) While I agree with the reasons for your change, the realization of your suggestion ends up being: res.extend(list(itertools.chain(*[self._includes[i].get_sections(key, attr, searched) for i in self._includes]))) Due to self._includes being a dict and the need to flatten the list before adding to res. I'm concerned that this is a loss in readability, even though it is certainly faster. - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4280/#review14033 --- On Dec. 19, 2014, 7:52 a.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4280/ --- (Updated Dec. 19, 2014, 7:52 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24474 https://issues.asterisk.org/jira/browse/ASTERISK-24474 Repository: Asterisk Description --- General improvements to reliability of conversion utility: 1) track default section of input file to allow parsing an include file that doesn't specify a [section] 2) informatively handle case of assignment with no section 3) correctly handle getting sections from included files 4) assume default bind of 0.0.0.0 5) gracefully handle missing portions of registration string 6) Denote steps of operation and confirm top level conf files being read/written as a convenience Diffs - /branches/12/contrib/scripts/sip_to_pjsip/sip_to_pjsip.py 429613 /branches/12/contrib/scripts/sip_to_pjsip/astconfigparser.py 429613 Diff: https://reviewboard.asterisk.org/r/4280/diff/ Testing --- Ran on config files from various sources to insure no exceptions occurred. Perused output to confirm appearance of converted input values. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4280: sip_to_pjsip: improved ability to parse input without exception
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4280/ --- (Updated Dec. 31, 2014, 9:10 a.m.) Review request for Asterisk Developers. Changes --- Addressed Matt's finding. Bugs: ASTERISK-24474 https://issues.asterisk.org/jira/browse/ASTERISK-24474 Repository: Asterisk Description --- General improvements to reliability of conversion utility: 1) track default section of input file to allow parsing an include file that doesn't specify a [section] 2) informatively handle case of assignment with no section 3) correctly handle getting sections from included files 4) assume default bind of 0.0.0.0 5) gracefully handle missing portions of registration string 6) Denote steps of operation and confirm top level conf files being read/written as a convenience Diffs (updated) - /branches/12/contrib/scripts/sip_to_pjsip/sip_to_pjsip.py 429613 /branches/12/contrib/scripts/sip_to_pjsip/astconfigparser.py 429613 Diff: https://reviewboard.asterisk.org/r/4280/diff/ Testing --- Ran on config files from various sources to insure no exceptions occurred. Perused output to confirm appearance of converted input values. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4256: testsuite: check for channel leak on failed blonde transfer
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4256/ --- (Updated Dec. 31, 2014, 12:01 p.m.) Review request for Asterisk Developers. Changes --- Addressed Matt's findings. Bugs: ASTERISK-24513 https://issues.asterisk.org/jira/browse/ASTERISK-24513 Repository: testsuite Description (updated) --- This test starts an attended transfer, converts to blonde mode by hanging up the transferer, and then fails the transfer by hanging up the transferee. Then after allowing the recall attempt to complete, checks to insure that there are not leaked channels. Improvements to channel_test_condition: count the actual channels listed in core show channels output to check for leaks. Also added unittest. Diffs (updated) - /asterisk/trunk/tests/bridge/tests.yaml 6149 /asterisk/trunk/tests/bridge/atxfer_fail_blonde/test-config.yaml PRE-CREATION /asterisk/trunk/tests/bridge/atxfer_fail_blonde/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/lib/python/asterisk/channel_test_condition.py 6149 Diff: https://reviewboard.asterisk.org/r/4256/diff/ Testing --- Currently fails while ASTERISK-24513 is not yet patched. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4256: testsuite: check for channel leak on failed blonde transfer
On Dec. 23, 2014, 9:41 a.m., Matt Jordan wrote: /asterisk/trunk/tests/bridge/atxfer_fail_blonde/test-config.yaml, line 35 https://reviewboard.asterisk.org/r/4256/diff/1/?file=69642#file69642line35 Minimum version should be the minimum expected Asterisk version that contains the fix. (1) Are we sure this should run against 11? (2) What minimum version of 13 should this include? I would have thought that the minimum version would be the version that had the features being tested, rather than turn off bug detection on earlier revisions just because it hadn't been fixed yet. Under those conditions, this is correct as all versions 11 forward have this feature - and I've tested it on 11 and it passes where 12 prior to my patches doesn't. - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4256/#review14036 --- On Dec. 31, 2014, 12:01 p.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4256/ --- (Updated Dec. 31, 2014, 12:01 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24513 https://issues.asterisk.org/jira/browse/ASTERISK-24513 Repository: testsuite Description --- This test starts an attended transfer, converts to blonde mode by hanging up the transferer, and then fails the transfer by hanging up the transferee. Then after allowing the recall attempt to complete, checks to insure that there are not leaked channels. Improvements to channel_test_condition: count the actual channels listed in core show channels output to check for leaks. Also added unittest. Diffs - /asterisk/trunk/tests/bridge/tests.yaml 6149 /asterisk/trunk/tests/bridge/atxfer_fail_blonde/test-config.yaml PRE-CREATION /asterisk/trunk/tests/bridge/atxfer_fail_blonde/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/lib/python/asterisk/channel_test_condition.py 6149 Diff: https://reviewboard.asterisk.org/r/4256/diff/ Testing --- Currently fails while ASTERISK-24513 is not yet patched. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4286: rtp_engine: avoid payload types above 127
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4286/ --- (Updated Dec. 31, 2014, 12:06 p.m.) Review request for Asterisk Developers. Changes --- Addressed Josh's findings. Bugs: ASTERISK-24367 https://issues.asterisk.org/jira/browse/ASTERISK-24367 Repository: Asterisk Description --- Valid payload type codes are between 0 and 127 to allow for being stored in 7 bits. During call setup, pjsip validates the SDP and will assert if it encounters an invalid payload type code (see pjmedia_sdp_validate() in pjmedia/src/pjmedia/sdp.c). This assert will be hit if a call is placed to a pjsip endpoint with allow=all set. To avoid this, the previous use 128 for the slin192 format has been changed to 95. Diffs (updated) - /trunk/main/rtp_engine.c 429845 Diff: https://reviewboard.asterisk.org/r/4286/diff/ Testing --- Tested with pjsip calls to allow=all configured extensions. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4286: rtp_engine: avoid payload types above 127
On Dec. 23, 2014, 7:41 a.m., Joshua Colp wrote: /trunk/main/rtp_engine.c, line 2193 https://reviewboard.asterisk.org/r/4286/diff/1/?file=69966#file69966line2193 Yeah, put this in the 96-127 rang instead. Scott Griepentrog wrote: Wouldn't that cause ast_rtp_engine_load_format() to fail because no dynamic rtp mapping is available? Joshua Colp wrote: I don't understand the question? There are unused payloads in the 96-127 range, I'd stick to that range for the reason oej provided as well as because I would not be surprised for implementations to consider that THE dynamic range and potentially have trouble outside of it. I was incorrect. There are other available payload numbers in the range. - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4286/#review14028 --- On Dec. 31, 2014, 12:06 p.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4286/ --- (Updated Dec. 31, 2014, 12:06 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24367 https://issues.asterisk.org/jira/browse/ASTERISK-24367 Repository: Asterisk Description --- Valid payload type codes are between 0 and 127 to allow for being stored in 7 bits. During call setup, pjsip validates the SDP and will assert if it encounters an invalid payload type code (see pjmedia_sdp_validate() in pjmedia/src/pjmedia/sdp.c). This assert will be hit if a call is placed to a pjsip endpoint with allow=all set. To avoid this, the previous use 128 for the slin192 format has been changed to 95. Diffs - /trunk/main/rtp_engine.c 429845 Diff: https://reviewboard.asterisk.org/r/4286/diff/ Testing --- Tested with pjsip calls to allow=all configured extensions. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4280: sip_to_pjsip: improved ability to parse input without exception
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4280/ --- (Updated Dec. 31, 2014, 12:13 p.m.) Review request for Asterisk Developers. Changes --- slightly improved with itervalues Bugs: ASTERISK-24474 https://issues.asterisk.org/jira/browse/ASTERISK-24474 Repository: Asterisk Description --- General improvements to reliability of conversion utility: 1) track default section of input file to allow parsing an include file that doesn't specify a [section] 2) informatively handle case of assignment with no section 3) correctly handle getting sections from included files 4) assume default bind of 0.0.0.0 5) gracefully handle missing portions of registration string 6) Denote steps of operation and confirm top level conf files being read/written as a convenience Diffs (updated) - /branches/12/contrib/scripts/sip_to_pjsip/sip_to_pjsip.py 429613 /branches/12/contrib/scripts/sip_to_pjsip/astconfigparser.py 429613 Diff: https://reviewboard.asterisk.org/r/4280/diff/ Testing --- Ran on config files from various sources to insure no exceptions occurred. Perused output to confirm appearance of converted input values. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4286: rtp_engine: avoid payload types above 127
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4286/ --- (Updated Dec. 31, 2014, 12:54 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 430164 Bugs: ASTERISK-24367 https://issues.asterisk.org/jira/browse/ASTERISK-24367 Repository: Asterisk Description --- Valid payload type codes are between 0 and 127 to allow for being stored in 7 bits. During call setup, pjsip validates the SDP and will assert if it encounters an invalid payload type code (see pjmedia_sdp_validate() in pjmedia/src/pjmedia/sdp.c). This assert will be hit if a call is placed to a pjsip endpoint with allow=all set. To avoid this, the previous use 128 for the slin192 format has been changed to 95. Diffs - /trunk/main/rtp_engine.c 429845 Diff: https://reviewboard.asterisk.org/r/4286/diff/ Testing --- Tested with pjsip calls to allow=all configured extensions. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 4302: bridge: channel ref leak after failed blond transfer
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4302/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24513 https://issues.asterisk.org/jira/browse/ASTERISK-24513 Repository: Asterisk Description --- After a blond transfer to a destination that then hangs up, the Local;1 channel would suffer a reference leak. This was supposed to be fixed by review 4262, but that caused an extra unreference on nominal blond transfer (somehow missed that in testing). This correction to the original problem reverts the prior change which had incorrectly mucked with moving the referenced transfer_target channel to recall_target in blond_nonfinal_enter(), and instead resolves the issue by insuring that the reference in recall_target is not lost by releasing it in recall_enter() before it is repalced on a successful dial. Diffs - /branches/13/main/bridge_basic.c 430163 Diff: https://reviewboard.asterisk.org/r/4302/diff/ Testing --- Both the blond_nominal and atxfer_fail_blond (r/4256) tests pass without error. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4280: sip_to_pjsip: improved ability to parse input without exception
On Dec. 17, 2014, 2:08 p.m., Mark Michelson wrote: /branches/12/contrib/scripts/sip_to_pjsip/sip_to_pjsip.py, lines 590-594 https://reviewboard.asterisk.org/r/4280/diff/1/?file=69819#file69819line590 I don't agree with this change. While I expect it to be rare, if someone had a working sip.conf file with no bindaddr or udpbindaddr present, this likely meant they had a TCP or TLS only setup. We shouldn't create a UDP transport in such a case. Removing the default udp transport would break the default of transport=transport-udp references for converted endpoints, as the assumption is that chan_sip would always have a default bind of 0.0.0.0:5060 even if not specified. I've instead changed it to default to a null bind value to hopefully indicate to the user that they may want to revise the transport settings manually. - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4280/#review13995 --- On Dec. 19, 2014, 7:52 a.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4280/ --- (Updated Dec. 19, 2014, 7:52 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24474 https://issues.asterisk.org/jira/browse/ASTERISK-24474 Repository: Asterisk Description --- General improvements to reliability of conversion utility: 1) track default section of input file to allow parsing an include file that doesn't specify a [section] 2) informatively handle case of assignment with no section 3) correctly handle getting sections from included files 4) assume default bind of 0.0.0.0 5) gracefully handle missing portions of registration string 6) Denote steps of operation and confirm top level conf files being read/written as a convenience Diffs - /branches/12/contrib/scripts/sip_to_pjsip/sip_to_pjsip.py 429613 /branches/12/contrib/scripts/sip_to_pjsip/astconfigparser.py 429613 Diff: https://reviewboard.asterisk.org/r/4280/diff/ Testing --- Ran on config files from various sources to insure no exceptions occurred. Perused output to confirm appearance of converted input values. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4280: sip_to_pjsip: improved ability to parse input without exception
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4280/ --- (Updated Dec. 19, 2014, 7:52 a.m.) Review request for Asterisk Developers. Changes --- Change default binding and improved help. Bugs: ASTERISK-24474 https://issues.asterisk.org/jira/browse/ASTERISK-24474 Repository: Asterisk Description --- General improvements to reliability of conversion utility: 1) track default section of input file to allow parsing an include file that doesn't specify a [section] 2) informatively handle case of assignment with no section 3) correctly handle getting sections from included files 4) assume default bind of 0.0.0.0 5) gracefully handle missing portions of registration string 6) Denote steps of operation and confirm top level conf files being read/written as a convenience Diffs (updated) - /branches/12/contrib/scripts/sip_to_pjsip/sip_to_pjsip.py 429613 /branches/12/contrib/scripts/sip_to_pjsip/astconfigparser.py 429613 Diff: https://reviewboard.asterisk.org/r/4280/diff/ Testing --- Ran on config files from various sources to insure no exceptions occurred. Perused output to confirm appearance of converted input values. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4279: chan_sip: Send CANCEL via proxy if CANCEL is to be sent after an UPDATE
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4279/#review14012 --- Ship it! Other than a few non-code issues, I ship thee. Did not trigger any SIP tagged test failures. http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_sip.c https://reviewboard.asterisk.org/r/4279/#comment24525 I would recommend appending to the comment block something like this: ASTERISK-24628: Send UPDATE to the same destination as CANCEL http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_sip.c https://reviewboard.asterisk.org/r/4279/#comment24526 Purely for readability, the sipmethod == SIP_UPDATE condition should be indented to the same level as SIP_ACK, since it is similarly nested under the same negation started on the line containing SIP_CANCEL. - Scott Griepentrog On Dec. 17, 2014, 12:11 p.m., kwemheuer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4279/ --- (Updated Dec. 17, 2014, 12:11 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24628 https://issues.asterisk.org/jira/browse/ASTERISK-24628 Repository: Asterisk Description --- Given three SIP phones (A, B, C), all communicating via a proxy (in this case OpenSIPs) with asterisk. A call is established between A and B. B sets the call on hold (A is hearing MOH) and dials the extension of C. While phone C is ringing, B transfers the call (A is hearing ringing of phone C, B is idle). On SIP there is a REFER to transfer the call. In case sendrpid=yes asterisk sends an UPDATE to phone C. This update is sent directly (not through the proxy). If someone (e.g. B) is trying to get the call back (through a directed pickup), asterisk sends a CANCEL to C. But this CANCEL is sent directly to C not through the proxy. The phone ignores this CANCEL according to the RFC3261 (Section 9.1). In other situations asterisk ensures, that a CANCEL is sent the same way the INVITE has been sent. In this case the previously sent UPDATE overwrites the address used later for the CANCEL. On the other hand the UPDATE has to be sent via the proxy too (IMHO). The call is in the ringing state and other endpoints maybe have to been updated too. The patch solves the issue. In function reqprep the UPDATE is handled the same way the CANCEL was (in case the state is in ringing state). Diffs - http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_sip.c 429698 Diff: https://reviewboard.asterisk.org/r/4279/diff/ Testing --- Patch is tested in the described scenario and solves the issue. Patched against 11.15.0. Thanks, kwemheuer -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4262: bridge: channel ref leak on blond_nonfinal_enter
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4262/ --- (Updated Dec. 19, 2014, 11:25 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 429826 Bugs: ASTERISK-24513 https://issues.asterisk.org/jira/browse/ASTERISK-24513 Repository: Asterisk Description --- After a blond transfer to a destination that then hangs up, the Local;1 channel would suffer a reference leak. This was due to blond_nonfinal_enter() resetting the props-transfer_target to NULL as it unref'd the channel for itself, later preventing the attended_transfer_properties_destructor() from releasing it's own reference, which was inherited during ast_bridge_impart with INDEPDENDENT. This change prevents props-transfer_target from being set to NULL, and thus eliminates the leak. Diffs - /branches/12/main/bridge_basic.c 429516 Diff: https://reviewboard.asterisk.org/r/4262/diff/ Testing --- Newly created test 'atxfer_fail_blonde' (https://reviewboard.asterisk.org/r/4256/) now passes. No other new test failures encountered. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 4286: rtp_engine: avoid payload types above 127
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4286/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24367 https://issues.asterisk.org/jira/browse/ASTERISK-24367 Repository: Asterisk Description --- Valid payload type codes are between 0 and 127 to allow for being stored in 7 bits. During call setup, pjsip validates the SDP and will assert if it encounters an invalid payload type code (see pjmedia_sdp_validate() in pjmedia/src/pjmedia/sdp.c). This assert will be hit if a call is placed to a pjsip endpoint with allow=all set. To avoid this, the previous use 128 for the slin192 format has been changed to 95. Diffs - /trunk/main/rtp_engine.c 429845 Diff: https://reviewboard.asterisk.org/r/4286/diff/ Testing --- Tested with pjsip calls to allow=all configured extensions. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 4280: sip_to_pjsip: improved ability to parse input without exception
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4280/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24474 https://issues.asterisk.org/jira/browse/ASTERISK-24474 Repository: Asterisk Description --- General improvements to reliability of conversion utility: 1) track default section of input file to allow parsing an include file that doesn't specify a [section] 2) informatively handle case of assignment with no section 3) correctly handle getting sections from included files 4) assume default bind of 0.0.0.0 5) gracefully handle missing portions of registration string 6) Denote steps of operation and confirm top level conf files being read/written as a convenience Diffs - /branches/12/contrib/scripts/sip_to_pjsip/sip_to_pjsip.py 429613 /branches/12/contrib/scripts/sip_to_pjsip/astconfigparser.py 429613 Diff: https://reviewboard.asterisk.org/r/4280/diff/ Testing --- Ran on config files from various sources to insure no exceptions occurred. Perused output to confirm appearance of converted input values. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4101: Channel Originate/Continue via ARI support for labels in dialplan is incomplete
On Dec. 17, 2014, 11:14 a.m., Mark Michelson wrote: /trunk/rest-api/api-docs/channels.json, line 374 https://reviewboard.asterisk.org/r/4101/diff/10/?file=69813#file69813line374 Why is this change present? greenfieldtech wrote: I changed this from int to long, for consistency sake. The other functions, referring to priority are all indicated as 'long'. The problem is, that even if it was inconsistent, as a part of the API it can't be changed without bumping the revision. This review should be focused on just adding your feature, and a separate attempt made to fix this. - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4101/#review13983 --- On Dec. 17, 2014, 6:39 a.m., greenfieldtech wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4101/ --- (Updated Dec. 17, 2014, 6:39 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24412 https://issues.asterisk.org/jira/browse/ASTERISK-24412 Repository: Asterisk Description --- This patch changes the current behavior of ARI, to allow channel originate/continue requests to be performed with labels as the priority, not only integer values. Diffs - /trunk/rest-api/api-docs/channels.json 429698 /trunk/res/res_ari_channels.c 429698 /trunk/res/ari/resource_channels.c 429698 /trunk/res/ari/resource_channels.h 429698 Diff: https://reviewboard.asterisk.org/r/4101/diff/ Testing --- Testing was performed by testing the following scenarios: 1. Originating a call to a numeric priority - works 2. Originating a call to a null priority - works 3. Originating a call to a label - works 4. Continue a call to a label - not tested yet Thanks, greenfieldtech -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4262: bridge: channel ref leak on blond_nonfinal_enter
On Dec. 12, 2014, 6:30 p.m., rmudgett wrote: /branches/12/main/bridge_basic.c, lines 2262-2264 https://reviewboard.asterisk.org/r/4262/diff/1/?file=69672#file69672line2262 Without looking further, the channel refs here seem to be mismatched. The recall_target is created and destroyed by the transfer code only, so that is a correct implementation. However the transfer_target has a reference to it that must be cleaned up on destruction of props- structure, so to set it to NULL at this point is incorrect. Because ast_channel_unref always returns NULL, the pattern of x=unref(x) only works where there is only one reference held to x in the context of x's lifetime, which is not the case here. - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4262/#review13959 --- On Dec. 12, 2014, 4:10 p.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4262/ --- (Updated Dec. 12, 2014, 4:10 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24513 https://issues.asterisk.org/jira/browse/ASTERISK-24513 Repository: Asterisk Description --- After a blond transfer to a destination that then hangs up, the Local;1 channel would suffer a reference leak. This was due to blond_nonfinal_enter() resetting the props-transfer_target to NULL as it unref'd the channel for itself, later preventing the attended_transfer_properties_destructor() from releasing it's own reference, which was inherited during ast_bridge_impart with INDEPDENDENT. This change prevents props-transfer_target from being set to NULL, and thus eliminates the leak. Diffs - /branches/12/main/bridge_basic.c 429516 Diff: https://reviewboard.asterisk.org/r/4262/diff/ Testing --- Newly created test 'atxfer_fail_blonde' (https://reviewboard.asterisk.org/r/4256/) now passes. No other new test failures encountered. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4262: bridge: channel ref leak on blond_nonfinal_enter
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4262/ --- (Updated Dec. 16, 2014, 3:09 p.m.) Review request for Asterisk Developers. Changes --- Added comment explaining the change. Bugs: ASTERISK-24513 https://issues.asterisk.org/jira/browse/ASTERISK-24513 Repository: Asterisk Description --- After a blond transfer to a destination that then hangs up, the Local;1 channel would suffer a reference leak. This was due to blond_nonfinal_enter() resetting the props-transfer_target to NULL as it unref'd the channel for itself, later preventing the attended_transfer_properties_destructor() from releasing it's own reference, which was inherited during ast_bridge_impart with INDEPDENDENT. This change prevents props-transfer_target from being set to NULL, and thus eliminates the leak. Diffs (updated) - /branches/12/main/bridge_basic.c 429516 Diff: https://reviewboard.asterisk.org/r/4262/diff/ Testing --- Newly created test 'atxfer_fail_blonde' (https://reviewboard.asterisk.org/r/4256/) now passes. No other new test failures encountered. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 4262: bridge: channel ref leak on blond_nonfinal_enter
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4262/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24513 https://issues.asterisk.org/jira/browse/ASTERISK-24513 Repository: Asterisk Description --- After a blond transfer to a destination that then hangs up, the Local;1 channel would suffer a reference leak. This was due to blond_nonfinal_enter() resetting the props-transfer_target to NULL as it unref'd the channel for itself, later preventing the attended_transfer_properties_destructor() from releasing it's own reference, which was inherited during ast_bridge_impart with INDEPDENDENT. This change prevents props-transfer_target from being set to NULL, and thus eliminates the leak. Diffs - /branches/12/main/bridge_basic.c 429516 Diff: https://reviewboard.asterisk.org/r/4262/diff/ Testing --- Newly created test 'atxfer_fail_blonde' (https://reviewboard.asterisk.org/r/4256/) now passes. No other new test failures encountered. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 4256: testsuite: check for channel leak on failed blonde transfer
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4256/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24513 https://issues.asterisk.org/jira/browse/ASTERISK-24513 Repository: testsuite Description --- This test starts an attended transfer, converts to blonde mode by hanging up the transferer, and then fails the transfer by hanging up the transferee. Then after allowing the recall attempts to expire, checks to insure that there are not leaked channels. Improvements to bridge_test_case: added support for setting reactor timeout Improvements to channel_test_condition: count the actual channels listed in core show channels output to check for leaks. Also added unittest. Diffs - /asterisk/trunk/tests/bridge/tests.yaml 6082 /asterisk/trunk/tests/bridge/atxfer_fail_blonde/test-config.yaml PRE-CREATION /asterisk/trunk/tests/bridge/atxfer_fail_blonde/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/lib/python/asterisk/channel_test_condition.py 6082 /asterisk/trunk/lib/python/asterisk/bridge_test_case.py 6082 Diff: https://reviewboard.asterisk.org/r/4256/diff/ Testing --- Currently fails while ASTERISK-24513 is not yet patched. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4182: core: avoid rasterisk crash due to long identifier
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4182/ --- (Updated Dec. 9, 2014, 2:46 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 429223 Repository: Asterisk Description --- When connecting to the remote console, an identifier string is first provided that consists of hostname/pid/version. This is parsed by the remote instance in a buffer allocated to only 80 bytes. It is possible for a combination of very long hostname and very long asterisk version number to be greater than 80 characters, causing the parsing to fall off the end of the allocated memory buffer and potentially crash. This change increases the buffer from 80 to 256 to significantly reduce that possibility. Diffs - /branches/13/main/asterisk.c 427948 Diff: https://reviewboard.asterisk.org/r/4182/diff/ Testing --- It stopped crashing on a repeated test I was running where the atoi of the version # happen to hit the end of the buffer. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4243: ari: Add the ability to specify an originator when originating calls.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4243/#review13919 --- Ship it! Ship It! /branches/13/res/ari/resource_channels.c https://reviewboard.asterisk.org/r/4243/#comment24443 I would suggest using a slightly more descriptive name for this struct that passes arguments to the dialing thread. - Scott Griepentrog On Dec. 5, 2014, 3:27 p.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4243/ --- (Updated Dec. 5, 2014, 3:27 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24552 https://issues.asterisk.org/jira/browse/ASTERISK-24552 Repository: Asterisk Description --- Currently when originating a call using ARI there is no way to specify an association of who is triggering the outbound dial. This is normally visible through the LinkedID. To allow this association to be present I've added the ability to specify the unique id of the channel doing the origination. If the channel exists on the system it will be examined and the linked id from it used. If the channel does not exist the request will be rejected with a 400. Diffs - /branches/13/rest-api/api-docs/channels.json 429026 /branches/13/res/res_ari_channels.c 429026 /branches/13/res/ari/resource_channels.c 429026 /branches/13/res/ari/resource_channels.h 429026 Diff: https://reviewboard.asterisk.org/r/4243/diff/ Testing --- Ran all existing ARI origination tests and confirmed they still pass. Performed calls manually and examined the resulting channels and CEL log to ensure they contain the linked ID of the originator. Thanks, Joshua Colp -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4244: ari: Add originate with originator test.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4244/#review13920 --- Ship it! Ship It! - Scott Griepentrog On Dec. 5, 2014, 3:44 p.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4244/ --- (Updated Dec. 5, 2014, 3:44 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24552 https://issues.asterisk.org/jira/browse/ASTERISK-24552 Repository: testsuite Description --- This test originates two channels. The first channel is originated without an originator but has a specified unique ID. The second channel originated uses the first channel as its originator. Upon completion the linked ID of the second channel is checked and confirmed to be the unique ID of the first channel. Diffs - /asterisk/trunk/tests/rest_api/channels/tests.yaml 6018 /asterisk/trunk/tests/rest_api/channels/originate_with_linkedid/test-config.yaml PRE-CREATION /asterisk/trunk/tests/rest_api/channels/originate_with_linkedid/configs/ast1/extensions.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/4244/diff/ Testing --- Ran test, it passes! Thanks, Joshua Colp -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4243: ari: Add the ability to specify a linkedId when originating calls.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4243/#review13908 --- I'm concerned that the use of ast_channel_internal_set_fake_ids() - which doesn't set the time fields of ast_channel_id - could result in possibly incorrect behavior during propagation. Granted, the supplied linkedid should always win over the originated channel, but then later that originated channel should win or not win in propagation based on the channel that the linkedid was originally copied from (whereas an unset time value would result in always being the oldest channel). At the very least, the time values need to be initalized, but preferably the entire ast_channel_id struct would be copied from the channel you want linkedid to come from. - Scott Griepentrog On Dec. 5, 2014, 2:28 p.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4243/ --- (Updated Dec. 5, 2014, 2:28 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24552 https://issues.asterisk.org/jira/browse/ASTERISK-24552 Repository: Asterisk Description --- Currently when originating a call using ARI there is no way to specify an association of who is triggering the outbound dial. This is normally visible through the LinkedID. To allow this association to be present I've added the ability to set LinkedID (optionally) when originating the call. Diffs - /branches/13/rest-api/api-docs/channels.json 429025 /branches/13/res/res_ari_channels.c 429025 /branches/13/res/ari/resource_channels.c 429025 /branches/13/res/ari/resource_channels.h 429025 Diff: https://reviewboard.asterisk.org/r/4243/diff/ Testing --- Ran all existing ARI origination tests and confirmed they still pass. Performed calls manually and examined the resulting channels and CEL log to ensure they contain the provided linked ID. Ran new test to confirm that the resulting CEL events contain the provided linked ID. Thanks, Joshua Colp -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4090: testsuite: add basic valgrind support
On Nov. 18, 2014, 8:18 a.m., Corey Farrell wrote: Please discard my findings, they are minor. I'd like to see this committed ASAP so further improvements can be made against it. I like your idea of adding an option to pass valgrind options, but it needs to first be set to write logs to the appropriate run_x/asty directory so there isn't a conflict between multiple asterisk instances. Then a simple parser added to check the log for critical issues that trigger a test failure, which then get included in the test results, even if in an abbreviated form. Not sure how best to pass leak info other than just leaving it in the log though. - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4090/#review13791 --- On Oct. 16, 2014, 4:23 p.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4090/ --- (Updated Oct. 16, 2014, 4:23 p.m.) Review request for Asterisk Developers. Repository: testsuite Description --- This adds very basic valgrind support, which is convenient for manual test runs but does not (yet) include support for more automated valgrind usage. 1) CLI flag '-V' enables valgrind (./runtests -V -t tests/test) 2) Minimal changes to testsuite, other improvements can be added later if desired 3) Valgrind output is picked up by error logging and shown after test run. 4) Unlike previous valgrind attempt, this one works fine on tests with multiple asterisk instances Diffs - /asterisk/trunk/runtests.py 5733 /asterisk/trunk/lib/python/asterisk/test_case.py 5733 /asterisk/trunk/lib/python/asterisk/asterisk.py 5733 Diff: https://reviewboard.asterisk.org/r/4090/diff/ Testing --- Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4090: testsuite: add basic valgrind support
On Oct. 16, 2014, 5:58 p.m., Kevin Harwell wrote: /asterisk/trunk/runtests.py, lines 461-462 https://reviewboard.asterisk.org/r/4090/diff/1/?file=68379#file68379line461 Not a huge fan of using an environment variable for this (could probably be convinced otherwise though). Maybe pass in as a param? While I concur with your distaste for using an environment variable like this, it has several advantages: 1) transparently gets passed to lib/python/asterisk/asterisk.py even if a custom run-test python application is being used (doesn't require rewriting existing tests) 2) tests written in other langauges can test for the flag (although I don't see a need for this happening) 3) It is possible to set the environment variable before calling ./runtests.py to trigger valgrind usage (although this will be more useful later when automated failure detection is added). - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4090/#review13561 --- On Oct. 16, 2014, 4:23 p.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4090/ --- (Updated Oct. 16, 2014, 4:23 p.m.) Review request for Asterisk Developers. Repository: testsuite Description --- This adds very basic valgrind support, which is convenient for manual test runs but does not (yet) include support for more automated valgrind usage. 1) CLI flag '-V' enables valgrind (./runtests -V -t tests/test) 2) Minimal changes to testsuite, other improvements can be added later if desired 3) Valgrind output is picked up by error logging and shown after test run. 4) Unlike previous valgrind attempt, this one works fine on tests with multiple asterisk instances Diffs - /asterisk/trunk/runtests.py 5733 /asterisk/trunk/lib/python/asterisk/test_case.py 5733 /asterisk/trunk/lib/python/asterisk/asterisk.py 5733 Diff: https://reviewboard.asterisk.org/r/4090/diff/ Testing --- Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4090: testsuite: add basic valgrind support
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4090/ --- (Updated Nov. 18, 2014, 9:49 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 5942 Repository: testsuite Description --- This adds very basic valgrind support, which is convenient for manual test runs but does not (yet) include support for more automated valgrind usage. 1) CLI flag '-V' enables valgrind (./runtests -V -t tests/test) 2) Minimal changes to testsuite, other improvements can be added later if desired 3) Valgrind output is picked up by error logging and shown after test run. 4) Unlike previous valgrind attempt, this one works fine on tests with multiple asterisk instances Diffs - /asterisk/trunk/runtests.py 5733 /asterisk/trunk/lib/python/asterisk/test_case.py 5733 /asterisk/trunk/lib/python/asterisk/asterisk.py 5733 Diff: https://reviewboard.asterisk.org/r/4090/diff/ Testing --- Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3496: testsuite: add valgrind support
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3496/ --- (Updated Nov. 18, 2014, 9:53 a.m.) Status -- This change has been discarded. Review request for Asterisk Developers. Repository: testsuite Description --- This patch adds support for running Asterisk under Valgrind (said Val-Grinned) to check for memory allocation and reference problems. To use, add -V to ./runtests.py and get a cup of coffee. Any errors will be displayed in a stack trace format. Tests can also activate valgrind in the yaml configuration. By default, only confirmed leaks will be shown - thoses where all copies of the pointer to the allocation have been released. Additional leak checking can be enabled. Diffs - /asterisk/trunk/runtests.py 5100 /asterisk/trunk/lib/python/asterisk/valgrind.py PRE-CREATION /asterisk/trunk/lib/python/asterisk/test_config.py 5100 /asterisk/trunk/lib/python/asterisk/test_case.py 5100 /asterisk/trunk/lib/python/asterisk/asterisk.py 5100 /asterisk/trunk/configs/valgrind.supp PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3496/diff/ Testing --- Used to locate reference problems on Userevent issue, and incorporated several improvements over the prior attempt. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4090: testsuite: add basic valgrind support
On Nov. 18, 2014, 8:18 a.m., Corey Farrell wrote: Please discard my findings, they are minor. I'd like to see this committed ASAP so further improvements can be made against it. Scott Griepentrog wrote: I like your idea of adding an option to pass valgrind options, but it needs to first be set to write logs to the appropriate run_x/asty directory so there isn't a conflict between multiple asterisk instances. Then a simple parser added to check the log for critical issues that trigger a test failure, which then get included in the test results, even if in an abbreviated form. Not sure how best to pass leak info other than just leaving it in the log though. Corey Farrell wrote: Upon further thought, I'm not sure it's desirable to be able to add options through the command-line. Extra options read from ~/.valgrindrc, $VALGRIND_OPTS, ./.valgrindrc. So I think in most cases it would be best for people to create ./.valgrindrc. Maybe we can add that to svn:ignore for '.'. But as you said, getting the logs to run_x/asty dirs is more important. Once this change is submitted I will look at putting the logs in the appropriate dir. As for parsing the test results, I think this would require using --xml-file=, then using libxml and/or libxslt (through python, sorry I don't know the modules for this). Anything we do to parse the text-based valgrind output will be a fragile hack. Agreed (fragile hacks to be avoided). There is code for parsing the xml file log (among other things) in https://reviewboard.asterisk.org/r/3496/, although parts of it are a bit convoluted (it attempts to reduce the stack traces of multiple issues down to a single tree for easier human consumption). - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4090/#review13791 --- On Nov. 18, 2014, 9:49 a.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4090/ --- (Updated Nov. 18, 2014, 9:49 a.m.) Review request for Asterisk Developers. Repository: testsuite Description --- This adds very basic valgrind support, which is convenient for manual test runs but does not (yet) include support for more automated valgrind usage. 1) CLI flag '-V' enables valgrind (./runtests -V -t tests/test) 2) Minimal changes to testsuite, other improvements can be added later if desired 3) Valgrind output is picked up by error logging and shown after test run. 4) Unlike previous valgrind attempt, this one works fine on tests with multiple asterisk instances Diffs - /asterisk/trunk/runtests.py 5733 /asterisk/trunk/lib/python/asterisk/test_case.py 5733 /asterisk/trunk/lib/python/asterisk/asterisk.py 5733 Diff: https://reviewboard.asterisk.org/r/4090/diff/ Testing --- Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4182: core: avoid rasterisk crash due to long identifier
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4182/ --- (Updated Nov. 14, 2014, 9:12 a.m.) Review request for Asterisk Developers. Changes --- I also realized I should have gone further with the fix before posting it. I started fixing it another way but prefer Corey's suggestion to my idea. Repository: Asterisk Description --- When connecting to the remote console, an identifier string is first provided that consists of hostname/pid/version. This is parsed by the remote instance in a buffer allocated to only 80 bytes. It is possible for a combination of very long hostname and very long asterisk version number to be greater than 80 characters, causing the parsing to fall off the end of the allocated memory buffer and potentially crash. This change increases the buffer from 80 to 256 to significantly reduce that possibility. Diffs (updated) - /branches/13/main/asterisk.c 427813 Diff: https://reviewboard.asterisk.org/r/4182/diff/ Testing --- It stopped crashing on a repeated test I was running where the atoi of the version # happen to hit the end of the buffer. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4139: stun: fix size of attribute string to match rfc
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4139/ --- (Updated Nov. 14, 2014, 9:48 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 427874 Repository: Asterisk Description --- When sending a USERNAME attribute in an RTP STUN response, the implementation in append_attr_string() passes the actual length of the string instead of padding it up to a multiple of 4 bytes size as required by the RFC 3489. This patch adds separate variables for calculating the string and padded attribute lengths, and handles the proper padding. Diffs - /branches/1.8/main/stun.c 426921 Diff: https://reviewboard.asterisk.org/r/4139/diff/ Testing --- Since I don't have an easy way to generate RTP STUN messages, I've tested the change on a separate test bed, and submitted the patch to the reporter for confirmation. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4182: core: avoid rasterisk crash due to long identifier
On Nov. 14, 2014, 4:08 p.m., Corey Farrell wrote:
/branches/13/main/asterisk.c, line 3203
https://reviewboard.asterisk.org/r/4182/diff/2/?file=68987#file68987line3203
Does this actually initialize 256 bytes of '\0', or just initialize the
first byte?
Initializing a char array with or { 0 } sets the entire array to zero,
whereas the values are undefined otherwise.
On Nov. 14, 2014, 4:08 p.m., Corey Farrell wrote:
/branches/13/main/asterisk.c, lines 3220-3222
https://reviewboard.asterisk.org/r/4182/diff/2/?file=68987#file68987line3220
Space around '-'.
Also why was the return removed?
I have absolutely no idea how that happened other than the vim ghost.
- Scott
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4182/#review13777
---
On Nov. 14, 2014, 5:03 p.m., Scott Griepentrog wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4182/
---
(Updated Nov. 14, 2014, 5:03 p.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
When connecting to the remote console, an identifier string is first provided
that consists of hostname/pid/version. This is parsed by the remote instance
in a buffer allocated to only 80 bytes. It is possible for a combination of
very long hostname and very long asterisk version number to be greater than
80 characters, causing the parsing to fall off the end of the allocated
memory buffer and potentially crash.
This change increases the buffer from 80 to 256 to significantly reduce that
possibility.
Diffs
-
/branches/13/main/asterisk.c 427948
Diff: https://reviewboard.asterisk.org/r/4182/diff/
Testing
---
It stopped crashing on a repeated test I was running where the atoi of the
version # happen to hit the end of the buffer.
Thanks,
Scott Griepentrog
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4182: core: avoid rasterisk crash due to long identifier
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4182/ --- (Updated Nov. 14, 2014, 5:03 p.m.) Review request for Asterisk Developers. Changes --- Corrected unintended change. Repository: Asterisk Description --- When connecting to the remote console, an identifier string is first provided that consists of hostname/pid/version. This is parsed by the remote instance in a buffer allocated to only 80 bytes. It is possible for a combination of very long hostname and very long asterisk version number to be greater than 80 characters, causing the parsing to fall off the end of the allocated memory buffer and potentially crash. This change increases the buffer from 80 to 256 to significantly reduce that possibility. Diffs (updated) - /branches/13/main/asterisk.c 427948 Diff: https://reviewboard.asterisk.org/r/4182/diff/ Testing --- It stopped crashing on a repeated test I was running where the atoi of the version # happen to hit the end of the buffer. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 4182: core: avoid rasterisk crash due to long identifier
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4182/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- When connecting to the remote console, an identifier string is first provided that consists of hostname/pid/version. This is parsed by the remote instance in a buffer allocated to only 80 bytes. It is possible for a combination of very long hostname and very long asterisk version number to be greater than 80 characters, causing the parsing to fall off the end of the allocated memory buffer and potentially crash. This change increases the buffer from 80 to 256 to significantly reduce that possibility. Diffs - /branches/13/main/asterisk.c 427813 Diff: https://reviewboard.asterisk.org/r/4182/diff/ Testing --- It stopped crashing on a repeated test I was running where the atoi of the version # happen to hit the end of the buffer. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4101: Channel Originate/Continue via ARI support for labels in dialplan is incomplete
On Nov. 5, 2014, 11:43 a.m., Joshua Colp wrote: Matt brought it up that this is actually a backwards incompatible change - specifically changing priority into a string from an integer. What about having label as a separate argument that is optional? If present it's treated as a label and if not then the priority is used as normal. This allows labels to be used with no backwards incompatible changes and also makes it a bit more explicit from a developer side of what they want. greenfieldtech wrote: Wow, that was actually my initial idea almost 4 weeks ago - the only issue was that I was feeling it was truly unclean. If you look into Asterisk docs, priority and labels and normally mixed. For example, if you use Goto in dialplan - you can do Goto(context,exten,priority) or Goto(context,exten,label). I personally feel it should work exactly the same, I see no point in having two methodologies. In addition, what should be used if both are defined? which has the stronger priority here? I feel this will also bring much confusion into the mix. Again, I can easily revert my original code work, but in my opinion it is very much confusing. Anyone else would like to share their thought? I agree that it would be easier to include this patch if it did not alter the existing API, only add an additional optional field. - Scott --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4101/#review13690 --- On Nov. 5, 2014, 8:16 a.m., greenfieldtech wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4101/ --- (Updated Nov. 5, 2014, 8:16 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24412 https://issues.asterisk.org/jira/browse/ASTERISK-24412 Repository: Asterisk Description --- This patch changes the current behavior of ARI, to allow channel originate/continue requests to be performed with labels as the priority, not only integer values. Diffs - /trunk/rest-api/api-docs/channels.json 425359 /trunk/res/res_ari_channels.c 425359 /trunk/res/ari/resource_channels.c 425359 /trunk/res/ari/resource_channels.h 425359 Diff: https://reviewboard.asterisk.org/r/4101/diff/ Testing --- Testing was performed by testing the following scenarios: 1. Originating a call to a numeric priority - works 2. Originating a call to a null priority - works 3. Originating a call to a label - works 4. Continue a call to a label - not tested yet Thanks, greenfieldtech -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4147: config: BUG: Restore ability for non-templates to be used as base objects in config.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4147/#review13667 --- Ship it! Ship It! - Scott Griepentrog On Nov. 4, 2014, 11:06 a.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4147/ --- (Updated Nov. 4, 2014, 11:06 a.m.) Review request for Asterisk Developers and Scott Griepentrog. Bugs: ASTERISK-24487 https://issues.asterisk.org/jira/browse/ASTERISK-24487 Repository: Asterisk Description --- My recent refactor of config.c accidentally removed the capability for a object to inherit from a non-template object. The following was no longer valid and should have been: [mybase] something=else [myobj](mybase) This patch restores the capability to inherit from both template and non-template objects. Diffs - branches/12/main/config.c 427199 Diff: https://reviewboard.asterisk.org/r/4147/diff/ Testing --- Testsuite manager/config tests still pass. main/config unit tests still pass. Eyeball test using AMI/GetConfig passed. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4129: pjsip: clarify tls key file usage
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4129/ --- (Updated Oct. 31, 2014, 8:39 a.m.) Review request for Asterisk Developers. Changes --- Removed trailing whitespace masquerading as an evil red block Repository: Asterisk Description --- Although it is possible to use a .pem file in place of a .crt file in a pjsip tls configuration, pjsip doesn't read the private key from it. This updates the documentation to clearly state the need to include the priv_key_file=file.key even though a pem file can be used for cert_file. Diffs (updated) - /branches/12/res/res_pjsip.c 426754 /branches/12/configs/pjsip.conf.sample 426754 Diff: https://reviewboard.asterisk.org/r/4129/diff/ Testing --- Rebuilt with no documentation errors. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4119: pjsip: handle outbound unregister correctly
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4119/ --- (Updated Oct. 31, 2014, 11:24 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 426925 Bugs: ASTERISK-24411 https://issues.asterisk.org/jira/browse/ASTERISK-24411 Repository: Asterisk Description --- Patch from John Bigelow: This patch sets the status of the outbound registration to reflect when it has been unregistered. Since the registration is unregistered rather than stopped, the registration schedule remains active as before. The patch also updates the documentation of both the AMI and CLI commands. Diffs - /branches/12/res/res_pjsip_outbound_registration.c 426523 Diff: https://reviewboard.asterisk.org/r/4119/diff/ Testing --- Previously failing test channels/pjsip/registration/outbound/unregister/unauthed now passes. Other pjsip tests that were passing still pass. Thanks, Scott Griepentrog -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
