Re: [asterisk-dev] Should internal timer implementing session timers be stopped and restarted on response to re-invite?
Mark Michelson mmichel...@digium.com wrote:
Looking in my Asterisk 11 version of chan_sip.c, in start_session_timer(), the
first if block looks like this:
if (p-stimer-st_schedid -1) {
/* in the event a timer is already going, stop it */
ast_debug(2, Session timer stopped: %d - %s\n, p-stimer-st_schedid,
p-callid);
AST_SCHED_DEL_UNREF(sched, p-stimer-st_schedid,
dialog_unref(p, unref stimer-st_schedid from dialog));
}
So it appears that calling start_session_timer() should be stopping any
existing timers if they are active. Basically, restart_session_timer() is
redundant and could be removed.
Thanks. Now I know what I'm looking for it was fixed in
https://issues.asterisk.org/jira/browse/ASTERISK-16023 although it makes no
mention of mis-operation of session timers, and doesn't even mention that if
fixes leaking internal timers. The title was [patch] UDP ports not
freed/ports leaking.
BTS Holdings PLC - Registered office: BTS House, Manor Road, Wallington, SM6
0DD - Registered in England: 1517630
--
_
-- 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] Process change proposal: allowing new features and improvements into release branches
Matt and Community,
This looks great to me. I've tweaked the wiki page a little bit to better
reflect the branch merging since 1.8 is now EOL. Otherwise, I think this
meets a very nice balance between allowing features in to keep pace with
the industry and anarchy.
I look forward to next AstriCon to see what features ended up coming from
this earlier than the next LTS (I understand it could be in Standard as
well, but that is a little more wild west than perhaps people might be
comfortable with).
One of the things is that new features must be in new feature releases, and
that some sort of announcement needs to be made showing that a new feature
was added. I assume this would happen in the ChangeLog and the announcement
release that goes out for all Asterisk releases. If there isn't already
some sort of flag or naming scheme that indicates new features, perhaps
there should be so that it is easier to parse out for the announcements.
Perhaps some sort of naming tag in the commit message like [FEATURE] or
something.
Would it also make sense to require new features be listed in the CHANGES
file for the point release? I don't want to add lots of work here, but when
you're developing against a major version release and new features are
going to be added, it would be great to have a go to spot (rather than
browsing all the ChangeLogs) for new features in case those deploying want
to rebase their work against a new point release in order to get a set of
features.
Last thing, regarding the old text block of Asterisk 12 is Different that
explains that changes are allowed towards the goals of that version; I
think it would be a good thing to have another similar block of text for
Asterisk 14. While the new policy is certainly more relaxed than the no
new features, I also don't think it goes quite far enough for Standard
releases whereby larger architecture changes appeared to be allowed within
the Standard release, at least where required to support the lofty goals of
Asterisk 12. I understand Asterisk 14 goals haven't been finalized, but
this is something to keep in mind for this page in the future.
Perhaps you might want to add a placeholder for that type of thing, or
generate a more generalized block of text that says Standard releases may
have changes that appear more aggressive than the feature release policy
dictates, but must be towards the goals set out for the Standard release
during AstriDevCon.
Thanks to everyone involved in making Asterisk awesome.
Leif.
On 10 November 2014 17:57, Matthew Jordan mjor...@digium.com wrote:
At AstriDevCon, we spent a good amount of time discussing whether or
not we should allow new features or improvements to be made in release
branches. As I summarized previously [1]:
{quote}
Draft a policy for allowing improvements in release branches.
The Asterisk project today does not exist in the same world - and is
not the same project - as when the no new feature in release
branches policy was first employed. We have a rigorous peer review
process, multiple test suites, continuous integration infrastructure,
and more to help prevent regressions or other problems from occurring.
In addition, the world today is also moving faster, and we'd like to
make sure that Asterisk maintains pace with changes in the industry.
At the same time, we have to ensure that release branches are stable
and that a user can upgrade within a release branch and not worry
about behavioural changes. We feel we can strike that balance with the
right policy.
{quote}
Before everyone rejoices/panics too much, there are restrictions on
proposing a new feature or improvement to a release branch:
(1) Any new feature proposed for an existing release branch must have
suitable test coverage using either the Asterisk Test Suite, the
Asterisk Unit Test Framework, or both.
(2) The new feature or improvement must be backwards compatible with
the previous releases in those major versions. That is, users
upgrading from one point release to the next should not be aware of
any new feature or improvement unless they want to use said feature.
Some things that should not be changed naturally follow from this:
(2a) APIs that follow semantic versioning should not receive a major
version increase.
(2b) Configuration and database schemas can be added to or updated,
but users should not be required to update their configuration or
databases.
(3) Any new features or improvements must be included in the first
release candidate of a new version. First release candidate
announcements must be made to the asterisk-users mailing lists, with
at least a week of testing time allowed. If a new feature or
improvement is deemed to cause an inappropriate burden on end-users,
it must be removed from the release.
Hopefully, this strikes the right balance of allowing the project to
keep pace with end user's needs, without introducing substantial risk
into release branches.
The full text of the
Re: [asterisk-dev] Process change proposal: allowing new features and improvements into release branches
+1
This sounds more than reasonable to me.
Sent from my iPhone
On Nov 10, 2014, at 5:57 PM, Matthew Jordan mjor...@digium.com wrote:
At AstriDevCon, we spent a good amount of time discussing whether or
not we should allow new features or improvements to be made in release
branches. As I summarized previously [1]:
{quote}
Draft a policy for allowing improvements in release branches.
The Asterisk project today does not exist in the same world - and is
not the same project - as when the no new feature in release
branches policy was first employed. We have a rigorous peer review
process, multiple test suites, continuous integration infrastructure,
and more to help prevent regressions or other problems from occurring.
In addition, the world today is also moving faster, and we'd like to
make sure that Asterisk maintains pace with changes in the industry.
At the same time, we have to ensure that release branches are stable
and that a user can upgrade within a release branch and not worry
about behavioural changes. We feel we can strike that balance with the
right policy.
{quote}
Before everyone rejoices/panics too much, there are restrictions on
proposing a new feature or improvement to a release branch:
(1) Any new feature proposed for an existing release branch must have
suitable test coverage using either the Asterisk Test Suite, the
Asterisk Unit Test Framework, or both.
(2) The new feature or improvement must be backwards compatible with
the previous releases in those major versions. That is, users
upgrading from one point release to the next should not be aware of
any new feature or improvement unless they want to use said feature.
Some things that should not be changed naturally follow from this:
(2a) APIs that follow semantic versioning should not receive a major
version increase.
(2b) Configuration and database schemas can be added to or updated,
but users should not be required to update their configuration or
databases.
(3) Any new features or improvements must be included in the first
release candidate of a new version. First release candidate
announcements must be made to the asterisk-users mailing lists, with
at least a week of testing time allowed. If a new feature or
improvement is deemed to cause an inappropriate burden on end-users,
it must be removed from the release.
Hopefully, this strikes the right balance of allowing the project to
keep pace with end user's needs, without introducing substantial risk
into release branches.
The full text of the proposed process changes has been made to the
Software Configuration Management Policies page on the Asterisk wiki
[2], with the proposed text put side-by-side with the existing text
for comparison. If we reach a consensus that this is a good thing,
I'll remove the old text.
Thoughts? Concerns?
[1] http://lists.digium.com/pipermail/asterisk-dev/2014-October/071076.html
[2]
https://wiki.asterisk.org/wiki/display/AST/Software+Configuration+Management+Policies
--
Matthew Jordan
Digium, Inc. | Engineering Manager
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
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
--
_
-- 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] 4151: Fix compiler error when using ./configure --enable-dev-mode --enable-coverage
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4151/ --- (Updated Nov. 12, 2014, 7:44 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 427682 Bugs: ASTERISK-24502 https://issues.asterisk.org/jira/browse/ASTERISK-24502 Repository: Asterisk Description --- When coverage and dev-mode are enabled with DONT_OPTIMIZE, it causes build failure. The double compilation does a 'shadow' build of each file with output to /dev/null. Unfortunately when coverage is enabled, GCC tries writing to /dev/null.gcno (at least some versions do). This prevents the build from proceeding. This change prevents coverage from being enabled for the shadow build of all files. This involves using a separate variable to hold the CFLAGS for coverage, and adding it to the commands for all real builds. Diffs - /branches/11/Makefile.rules 427380 Diff: https://reviewboard.asterisk.org/r/4151/diff/ Testing --- Build with these options now works in Linux Mint 17 (gcc 4.8.2-19ubuntu1). 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
[asterisk-dev] [Code Review] 4163: Stasis: Fix StasisEnd message ordering
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4163/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24501 https://issues.asterisk.org/jira/browse/ASTERISK-24501 Repository: Asterisk Description --- This change corrects message ordering in cases where a channel-related message can be received after a Stasis/ARI application has received the StasisEnd message. The StasisEnd message was being passed to applications directly without waiting for the channel topic to empty. As a result of this fix, other bugs were also identified and fixed: * StasisStart messages were also being sent directly to apps and are now routed through the stasis message bus properly * Masquerade monitor datastores were being removed at the incorrect time in some cases and were causing StasisEnd messages to not be sent * General refactoring where necessary for the above * Unsubscription on StasisEnd timing changes to prevent additional messages from following the StasisEnd when they shouldn't A channel sanitization function pointer was added to reduce processing and AO2 lookups This also required minor changes to tests using AriTestObject or its subclasses since StasisEnd is no longer reliably received before the test shuts the websocket down. This is due to the AriTestObject relying on AMI events to decide when the test is over which won't necessarily come in at the same time as the corresponding ARI events since they arrive via two different transports. Diffs - branches/12/res/stasis/stasis_bridge.c 427539 branches/12/res/stasis/app.c 427539 branches/12/res/stasis/app.h 427539 branches/12/res/res_stasis.c 427539 branches/12/include/asterisk/stasis_app.h 427539 branches/12/include/asterisk/stasis.h 427539 Diff: https://reviewboard.asterisk.org/r/4163/diff/ Testing --- Ran all the REST API tests to verify that they passed. Thanks, opticron -- _ -- 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] 4162: pbx: Fix crash in off-nominal when add_priority encounters a failure.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4162/ --- (Updated Nov. 12, 2014, 10:10 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 427709 Bugs: ASTERISK-2 https://issues.asterisk.org/jira/browse/ASTERISK-2 Repository: Asterisk Description --- When adding an extension it is initially created in the context hash table and also in the pattern tree. This occurs before a priority is added. If afterwards the priority is added and it encounters a failure the entire operation returns an error. In this case the extension is freed but is never removed from the context hash table or the pattern tree. Subsequent access may encounter this invalid extension causing it to access freed memory. This change no longer makes add_priority responsible for cleaning up the extension and leaves it up to the caller. The caller now removes the extension from the context hash table and pattern tree and then frees the extension. Diffs - /branches/11/main/pbx.c 427541 Diff: https://reviewboard.asterisk.org/r/4162/diff/ Testing --- Before patch: Subscribed to a hint twice - once with no spaces in it, second time with spaces in it. Running in valgrind showed memory access after being freed. After patch: Subscribed to a hint twice - once with no spaces in it, second time with spaces in it. Running in valgrind showed no memory access issues. 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] 4164: res_pjsip_outbound_registration: stack overflow when using non-default sorcery wizard
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4164/#review13734 --- Ship it! Ship It! - Mark Michelson On Nov. 11, 2014, 8:56 p.m., Kevin Harwell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4164/ --- (Updated Nov. 11, 2014, 8:56 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24514 https://issues.asterisk.org/jira/browse/ASTERISK-24514 Repository: Asterisk Description --- When using a non-default sorcery wizard (in this instance realtime) for outbound registrations and after adding in an appropriate call to ast_sorcery_apply_config() (since it is missing) Asterisk will crash after a stack overflow occurs due to the code infinitely recursing. The fix entails removing the outbound registration state dependency from the outbound registration sorcery object and instead keeping an in memory container that can be used to lookup the state when needed. Diffs - branches/12/res/res_pjsip_outbound_registration.c 427675 Diff: https://reviewboard.asterisk.org/r/4164/diff/ Testing --- On top of running the current testsuite tests I also manually tested various configurations and scenarios using a static configuration file as well as dynamic realtime. Verified that the crash no longer occurs and the potentially affected functionality works as expected (for instance, cli/ami commands, module [re]loading, and manual unregistration). 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] 4155: PJSIP: Allow contact rewriting to fall back for reliable transports
On Nov. 10, 2014, 8:24 a.m., Joshua Colp wrote: branches/12/res/res_pjsip_nat.c, line 230 https://reviewboard.asterisk.org/r/4155/diff/1/?file=68767#file68767line230 Just a question - is this already on the dialog? (Do you need to clone it?) Joshua Colp wrote: Or can you steal it like a thief. The uri is allocated on the rdata and not on the dialog, so the clone is necessary. - opticron --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4155/#review13717 --- On Nov. 6, 2014, 9:49 a.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4155/ --- (Updated Nov. 6, 2014, 9:49 a.m.) Review request for Asterisk Developers and Joshua Colp. Bugs: ASTERISK-24486 https://issues.asterisk.org/jira/browse/ASTERISK-24486 Repository: Asterisk Description --- This modifies contact rewriting to revert to the original contact URI for a dialog when persistent transports shut down. Some calls can enter a condition where their contact URI from the initial incoming invite was rewritten for a reliable transport, but that transport has been shut down due to inactivity. Such reliable transports can not be re-established since the remote host was never listening on the associated port. In cases such as these, it is useful to be able to fall back to the original contact URI since it might be accessible and allow the call to continue normally. Diffs - branches/12/res/res_pjsip_nat.c 425222 Diff: https://reviewboard.asterisk.org/r/4155/diff/ Testing --- Verified that this allowed the call to operate normally despite the original reliable connection being torn down where this situation was experienced. Thanks, opticron -- _ -- 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] 4163: Stasis: Fix StasisEnd message ordering
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4163/#review13736 --- As a general comment, I suggest adding errror messages for all off-nominal paths when it comes to sending StasisStart or StasisEnd events. If these messages don't get sent, it's really helpful to be able to inspect the logs and figure out what went wrong. - Mark Michelson On Nov. 12, 2014, 3:58 p.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4163/ --- (Updated Nov. 12, 2014, 3:58 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24501 https://issues.asterisk.org/jira/browse/ASTERISK-24501 Repository: Asterisk Description --- This change corrects message ordering in cases where a channel-related message can be received after a Stasis/ARI application has received the StasisEnd message. The StasisEnd message was being passed to applications directly without waiting for the channel topic to empty. As a result of this fix, other bugs were also identified and fixed: * StasisStart messages were also being sent directly to apps and are now routed through the stasis message bus properly * Masquerade monitor datastores were being removed at the incorrect time in some cases and were causing StasisEnd messages to not be sent * General refactoring where necessary for the above * Unsubscription on StasisEnd timing changes to prevent additional messages from following the StasisEnd when they shouldn't A channel sanitization function pointer was added to reduce processing and AO2 lookups This also required minor changes to tests using AriTestObject or its subclasses since StasisEnd is no longer reliably received before the test shuts the websocket down. This is due to the AriTestObject relying on AMI events to decide when the test is over which won't necessarily come in at the same time as the corresponding ARI events since they arrive via two different transports. Diffs - branches/12/res/stasis/stasis_bridge.c 427539 branches/12/res/stasis/app.c 427539 branches/12/res/stasis/app.h 427539 branches/12/res/res_stasis.c 427539 branches/12/include/asterisk/stasis_app.h 427539 branches/12/include/asterisk/stasis.h 427539 Diff: https://reviewboard.asterisk.org/r/4163/diff/ Testing --- Ran all the REST API tests to verify that they passed. Thanks, opticron -- _ -- 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] 4167: Allow mis-dialed DTMF-initiated transfers to be re-attempted.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4167/ --- (Updated Nov. 12, 2014, 6:32 p.m.) Review request for Asterisk Developers. Changes --- It was conveyed off-list that if I'm messing with this area, the sound files to play should be configurable as well. It was easy enough to implement, so I added the transferretrysound and transferinvalidsound options to be played. I'm open to different names for any of the options introduced in this changeset. Repository: Asterisk Description --- There have been complaints about the fact that when a DTMF transfer is initiated, and an incorrect number is dialled, the prompt says Please try again but then doesn't give the transferer a chance to do so without re-initiating the transfer. This patch adds a features.conf option called transferdialattempts (yes, the name is gross, but it fits in with the convention of other options in features.conf). This option controls how many attempts a transferer has to dial a valid extension before being booted back out to the call with the transferee. The option defaults to 3. As with other options from features.conf, this can also be set using the FEATURE() dialplan function so that it may be changed per-call if desired. Diffs (updated) - /trunk/main/features_config.c 427676 /trunk/main/bridge_basic.c 427676 /trunk/include/asterisk/features_config.h 427676 Diff: https://reviewboard.asterisk.org/r/4167/diff/ Testing --- See /r/4168 Thanks, Mark Michelson -- _ -- 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] 4163: Stasis: Fix StasisEnd message ordering
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4163/ --- (Updated Nov. 12, 2014, 12:56 p.m.) Review request for Asterisk Developers. Changes --- Added some error messages for critical failure paths. Bugs: ASTERISK-24501 https://issues.asterisk.org/jira/browse/ASTERISK-24501 Repository: Asterisk Description --- This change corrects message ordering in cases where a channel-related message can be received after a Stasis/ARI application has received the StasisEnd message. The StasisEnd message was being passed to applications directly without waiting for the channel topic to empty. As a result of this fix, other bugs were also identified and fixed: * StasisStart messages were also being sent directly to apps and are now routed through the stasis message bus properly * Masquerade monitor datastores were being removed at the incorrect time in some cases and were causing StasisEnd messages to not be sent * General refactoring where necessary for the above * Unsubscription on StasisEnd timing changes to prevent additional messages from following the StasisEnd when they shouldn't A channel sanitization function pointer was added to reduce processing and AO2 lookups This also required minor changes to tests using AriTestObject or its subclasses since StasisEnd is no longer reliably received before the test shuts the websocket down. This is due to the AriTestObject relying on AMI events to decide when the test is over which won't necessarily come in at the same time as the corresponding ARI events since they arrive via two different transports. Diffs (updated) - branches/12/res/stasis/stasis_bridge.c 427539 branches/12/res/stasis/app.c 427539 branches/12/res/stasis/app.h 427539 branches/12/res/res_stasis.c 427539 branches/12/include/asterisk/stasis_app.h 427539 branches/12/include/asterisk/stasis.h 427539 Diff: https://reviewboard.asterisk.org/r/4163/diff/ Testing --- Ran all the REST API tests to verify that they passed. Thanks, opticron -- _ -- 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] 4173: bridge: Protect bridge channel when changing state and make it smarter
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4173/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24440 https://issues.asterisk.org/jira/browse/ASTERISK-24440 Repository: Asterisk Description --- This is a patch originally done by Richard. This is only applicable to Asterisk 11. It adds locking around bridge channel state changes and makes it smarter, so that an important state (such as hangup) can't be undone. Without this change under high load ConfBridge may keep channels around when it shouldn't. Diffs - /branches/11/main/bridging.c 427520 Diff: https://reviewboard.asterisk.org/r/4173/diff/ Testing --- Ran sipp against chan_sip with ConfBridge at heavy CPS and confirmed that channels I would expect to be terminated were. 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] 4160: chan_sip: Fix theoretical leak of p-refer
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4160/ --- (Updated Nov. 12, 2014, 3:28 p.m.) Review request for Asterisk Developers. Changes --- Make sip_refer_alloc destroy any previous p-refer. Bugs: ASTERISK-15242 https://issues.asterisk.org/jira/browse/ASTERISK-15242 Repository: Asterisk Description --- If transmit_refer is called when p-refer is already allocated, it will leak the previous allocation. I checked for all occurrences of sip_refer_alloc, found that transmit_refer was the only caller that didn't check p-refer first. This change moves the check for !p-refer to sip_refer_alloc. I made transmit_refer destroy any previous p-refer so it will have a clean structure after reallocation like it does currently. Unsure if it's needed, but the little bit of extra processing is worth keeping this fix low risk. The change is slightly different in 12+, as p-refer-refer_call only exists in 11. Diffs (updated) - /branches/11/channels/chan_sip.c 427685 Diff: https://reviewboard.asterisk.org/r/4160/diff/ Testing (updated) --- tests/channels/SIP against 11 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
[asterisk-dev] [Code Review] 4175: Fix race condition where identical SIP requests are processed by multiple threads (Asterisk 13)
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4175/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- See /r/4174 for details about the issue that is being fixed. The main difference here is that since res_pjsip_pubsub.c was refactored between Asterisk 12 and Asterisk 13, the fix here is much simpler. Instead of having to save the result of dialog creation on the rdata's mod_data array, we can instead just add an extra output parameter when creating the subscription tree to determine if dialog creation was successful. Diffs - /branches/13/res/res_pjsip_session.c 427675 /branches/13/res/res_pjsip_pubsub.c 427675 /branches/13/res/res_pjsip.c 427675 /branches/13/main/features.c 427675 /branches/13/include/asterisk/res_pjsip.h 427675 Diff: https://reviewboard.asterisk.org/r/4175/diff/ Testing --- I reran the subscription presence tests to be sure that they are not affected by this change. Thanks, Mark Michelson -- _ -- 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] 4174: Fix race condition where identical SIP requests are processed by multiple threads.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4174/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- During testing, an odd situation was encountered. In the test, a phone sent an INVITE to Asterisk. A half second later, after receiving no response, the phone retransmits the INVITE to Asterisk. About this time, Asterisk starts to process both incoming INVITEs at the same time in separate threads. In thread 1, a dialog is successfully created, a 100 Trying response is sent, and the call is sent into the dialplan. In thread 2, the dialog cannot be created because thread 1 has already created a transaction in PJSIP with the same details. The collision results in thread 2 sending a 500 response to the phone. At this point, the phone has received an error final response, so the phone assumes the call is failed. However, Asterisk has a successful dialog going, still, so Asterisk continues on with the call. This results in some fun situations. Luckily, the situations haven't proven fatal for Asterisk, but they are very confusing for people involved in the calls. The solution proposed to fix this problem is to not respond to incoming requests if attempting to create a transaction results in the PJ_EEXISTS error. The logic is that if PJ_EEXISTS is returned, that means that elsewhere, we have already successfully created a transaction for this request and we can safely ignore this one. After auditing the code, the only places that required changes were the places that created dialogs based on incoming requests. Places that create out-of-dialog stateful responses were not reacting to errors by sending stateless responses. The actual change implemented here is to modify ast_sip_create_dialog_uas() to take an additional parameter that is the status returned from PJSIP when attempting to create the dialog. This way, we can react accordingly if the dialog cannot be created. The Asterisk 12 changes are presented here. The Asterisk 13 changes are on /r/4175 Diffs - /branches/12/res/res_pjsip_session.c 427735 /branches/12/res/res_pjsip_pubsub.c 427735 /branches/12/res/res_pjsip.c 427735 /branches/12/include/asterisk/res_pjsip.h 427735 Diff: https://reviewboard.asterisk.org/r/4174/diff/ Testing --- I ran the testsuite nominal incoming calls tests and the presence subscription tests to be sure that this change did not adversely affect them. They still pass. Thanks, Mark Michelson -- _ -- 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] 4165: res_pjsip_config_wizard: Allow streamlined configuration of common pjsip scenarios.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4165/ --- (Updated Nov. 12, 2014, 3:08 p.m.) Review request for Asterisk Developers, Joshua Colp, Marquis, and Mark Michelson. Changes --- Add some logging. Fixed a leak and a double free. Fixed a a variable initialization. Cleaned up some names and formatting. Repository: Asterisk Description --- The PJSIP Configuration Wizard allows for creation of simple pjsip scenarios like phone or trunk without having to directly specify individual endpoints, aors, auths, identifies or registrations. The easiest way to demonstrate this is with an example or two from pjsip.conf.sample... ;EXAMPLE WIZARD CONFIGURATION FOR A PHONE=== ; This config would create an endpoint, aor with dynamic contact, inbound ; auth and a phoneprov object. [myphone] type = wizard accepts_auth = yes accepts_registrations = yes transport = ipv4 aor/max_contacts = 1 inbound_auth/username = testname inbound_auth/password = test password endpoint/allow = ulaw endpoint/context = default phoneprov/MAC = 001122aa4455 phoneprov/PROFILE = profile1 ;EXAMPLE WIZARD CONFIGURATION FOR AN ITSP TRUNK= ; This ITSP has 2 servers available and requires registration. ; This config would create an endpoint, an aor with 2 static contacts, an outbound ; auth, an identify with 2 matches, and 2 registrations. [mytrunk] type = wizard sends_auth = yes sends_registrations = yes transport = ipv4 ; The number of remote_hosts drives the number of contacts, matches and registrations. remote_hosts = sip1.myitsp.com:5060,sip2.myitsp.com:5060 outbound_auth/username = testname outbound_auth/password = test password endpoint/allow = ulaw endpoint/context = default pjsip.conf.sample has more details. The history of the wizard approach can be found in the following 2 threads... http://lists.digium.com/pipermail/asterisk-dev/2014-September/070426.html http://lists.digium.com/pipermail/asterisk-dev/2014-October/070616.html THEORY OF OPERATION: N.B.: There are two kinds of wizards referenced here: The existing sorcery wizards which provide the storage back ends like res_sorcery_config and res_sorcery_realtime, and this new pjsip config wizard. The new module implements both a new sorcery wizard and the pjsip wizard. The wizard is implemented in a single module but did require a few tweaks to other res_pjsip modules and sorcery itself. There are 2 parts to this module, the config wizard and the sorcery wizard. When the module loads, the config wizard part loads pjsip.conf and pulls in all of the 'wizard' object types. Assuming they're all semantically valid, the module then registers itself as a sorcery wizard. The sorcery wizard part provides a sorcery back end almost exactly like res_sorcery_memory. A change was made to each of the pjsip modules so that as they load, they apply wizard mappings from not only sorcery.conf and pjsip.conf, but also from the new 'pjsip_wizard' sorcery wizard. The result is that then they call ast_sorcery_load_object, sorcery also pulls objects from the pjsip_wizard after it's pulled them from pjsip.conf. That in turn causes pjsip_wizard to create auths, aors, endpoints, identifies, registrations and phoneprovs from the data it collected earlier. pjsip is non e the wiser about where the objects came from and all AMI, ARI, CLI etc. operate as normal. SUMMARY OF CHANGES MADE: * The new res_pjsip_config_wizard module was created. * An existing internal sorcery api was exposed as ast_sorcery_apply_wizard_mapping to allow the addition of a new wizard to an object type. The underlying plumbing was already there. * config_auth, location, pjsip_configuration, res_pjsip_endpoint_identifier_ip, res_pjsip_outbound_registration and res_pjsip_phoneprov_provider were all modified to call ast_sorcery_apply_wizard_mapping after calling ast_sorcery_apply_default. * res_pjsip_phoneprov_provider needed a little more work to be compatible. RELOADABILITY: I've done my best to make the new module properly unload and reload but there are issues with the rest of the pjsip modules that make this tricky. Further work will be needed for consistent module operations across all of pjsip. BACKWARDS COMPATIBILITY: This module does not change any existing functionality. Discrete objects and wizards may both exist in a single pjsip.conf file. If the res_pjsip_config_wizard module isn't loaded, the wizard types will just be ignored. If the module is loaded but there are no wizard types in pjsip.conf, it stays loaded but empty. Once created by the wizard, pjsip objects are indistinguishable from ones created discretely. OTHER: Even though the new module provides a sorcery wizard, it can get its own wizard objects from other
Re: [asterisk-dev] [Code Review] 4163: Stasis: Fix StasisEnd message ordering
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4163/#review13737 --- Ship it! Ship It! - Mark Michelson On Nov. 12, 2014, 6:56 p.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4163/ --- (Updated Nov. 12, 2014, 6:56 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24501 https://issues.asterisk.org/jira/browse/ASTERISK-24501 Repository: Asterisk Description --- This change corrects message ordering in cases where a channel-related message can be received after a Stasis/ARI application has received the StasisEnd message. The StasisEnd message was being passed to applications directly without waiting for the channel topic to empty. As a result of this fix, other bugs were also identified and fixed: * StasisStart messages were also being sent directly to apps and are now routed through the stasis message bus properly * Masquerade monitor datastores were being removed at the incorrect time in some cases and were causing StasisEnd messages to not be sent * General refactoring where necessary for the above * Unsubscription on StasisEnd timing changes to prevent additional messages from following the StasisEnd when they shouldn't A channel sanitization function pointer was added to reduce processing and AO2 lookups This also required minor changes to tests using AriTestObject or its subclasses since StasisEnd is no longer reliably received before the test shuts the websocket down. This is due to the AriTestObject relying on AMI events to decide when the test is over which won't necessarily come in at the same time as the corresponding ARI events since they arrive via two different transports. Diffs - branches/12/res/stasis/stasis_bridge.c 427539 branches/12/res/stasis/app.c 427539 branches/12/res/stasis/app.h 427539 branches/12/res/res_stasis.c 427539 branches/12/include/asterisk/stasis_app.h 427539 branches/12/include/asterisk/stasis.h 427539 Diff: https://reviewboard.asterisk.org/r/4163/diff/ Testing --- Ran all the REST API tests to verify that they passed. Thanks, opticron -- _ -- 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] 4158: rtp_engine: Fix crash when endpoints send more RTCP report info blocks then we can handle
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4158/ --- (Updated Nov. 12, 2014, 5:59 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 427762 Bugs: ASTERISK-24489 and ASTERISK-24498 https://issues.asterisk.org/jira/browse/ASTERISK-24489 https://issues.asterisk.org/jira/browse/ASTERISK-24498 Repository: Asterisk Description --- Asterisk - in res_rtp_asterisk - only understands a single RTCP report info block. When the RTCP information was refactored in the RTP Engine to be pushed over the Stasis message bus, I put in the hooks into the engine to handle multiple RTCP report info blocks, in the hope that a future RTP implementation would be able to provide that data. Unfortunately, res_rtp_asterisk has a tendency to lie: (1) It will send RTCP reports with a reception_report_count greater than 1 (which is pulled directly from the RTCP packet itself, so that part is correct) (2) It will only provide a single report block When the rtp_engine goes to convert this to a JSON blob, hilarity ensues as it looks for a report block that doesn't exist. This patch updates the rtp_engine to be a bit more skeptical about what it is presented with. While this could also be fixed in res_rtp_asterisk, I prefer to fix it in the engine for two reasons: (1) The engine is designed to work with multiple RTP implementation, and hence having it be more robust is a good thing (tm) (2) res_rtp_asterisk's handling of RTCP information is fun. It should report the correct reception_report_count; ideally it should also be giving us all of the blocks - but it is *definitely* not designed to do that. Going down that road is a non-trivial effort. Diffs - /branches/12/main/rtp_engine.c 427539 Diff: https://reviewboard.asterisk.org/r/4158/diff/ Testing --- Issue reporter tested and verified that they no longer crash. 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
