It is possible when processing a SIP REGISTER request to have two
threads end up creating contact_status structures in sorcery.
contact_status is created using a "find or create" function. If two
threads call into this at the same time, each thread will fail to find
an existing contact_status, and so both will end up creating a new
contact status.
During testing, we would see sporadic failures because the
PJSIP_CONTACT() dialplan function would operate on a different
contact_status than what had been updated by res_pjsip/pjsip_options.
The fix here is two-fold:
1) The "find or create" function for contact_status now has a lock
around the entire operation. This way, if two threads attempt the
operation simultaneously, the first to get there will create the object,
and the second will find the object created by the first thread.
2) res_sorcery_memory has had its create callback updated so that it
will not allow for objects with duplicate IDs to be created.
Change-Id: I55b1460ff1eb0af0a3697b82d7c2bac9f6af5b97
A problem arose when testing the AMI subscription listing actions where it
was possible for a subscription that had not been fully initialized to be
listed. This was problematic as the underlying listing code would crash.
This change makes it so the subscription tree is fully set up before it is
added to the list of subscriptions. This ensures that when the listing actions
get the subscription it is valid.
ASTERISK-25738 #close
Change-Id: Iace2b13641c31bbcc0d43a39f99aba1f340c0f48
load_module was just too hairy with every step having to clean up all
previous steps on failure.
Some of the pjproject init calls have now been moved to a separate
load_pjsip function and the unload_pjsip function was enhanced to clean
up everything if an error happened at any stage of the load process.
In the process, a bunch of missing pj_shutdowns, serializer_pool_shutdowns
and ast_threadpool_shutdowns were also corrected.
Change-Id: I5eec711b437c35b56605ed99537ebbb30463b302
* heap-use-after-free happens when we free "cfg"
but then use "value" which refers to it
* A memory leak occurs because in some cases
it is not released "defaults"
ASTERISK-25721 #close
Reported by: Badalyan Vyacheslav
Tested by: Badalyan Vyacheslav
Change-Id: I3807d3f4726df6864430ec144cf6265d3f538469
Pjproject has deprecated pjsip_dlg_create_uas in 2.5 and replaced it with
pjsip_dlg_create_uas_and_inc_lock which, as the name implies, automatically
increments the lock on the returned dialog. To account for this, configure.ac
now detects the presence of pjsip_dlg_create_uas_and_inc_lock and res_pjsip.c
has an #ifdef HAVE_PJSIP_DLG_CREATE_UAS_AND_INC_LOCK to decide whether to use
the original call or the new one. If the new one was used, the ref count is
decremented before returning.
ASTERISK-25751 #close
Reported-by Josh Colp
Change-Id: I1be776b94761df03bd0693bc7795a75682615ca8
Attempting to load a transport from realtime was forcing asterisk into an
infinite recursion loop. The first thing transport_apply did was to do a
sorcery retrieve by id for an existing transport of the same name. For files,
this just returns the previous object from res_sorcery_config's internal
container, if any. For realtime, the res_sourcery_realtime driver looks in the
database and finds the existing row but now it has to rehydrate it into a
sorcery object which means calling... transport_apply. And so it goes.
The main issue with loading from realtime (apart from the loop) was that
transport stores structures and pointers directly in the ast_sip_transport
structure instead of the separate ast_transport_state structure. This patch
separates those items into the ast_sip_transport_state structure. The pattern
is roughly the same as res_pjsip_outbound_registration.
Although all current usages of ast_sip_transport and ast_sip_transport_state
were modified to use the new ast_sip_get_transport_state API, the original
items are left in ast_sip_transport and kept updated to maintain ABI
compatability for third-party modules. They are marked as deprecated and
noted that they're now in ast_sip_transport_state.
ASTERISK-25606 #close
Reported-by: Martin Moučka
Change-Id: Ic7a836ea8e786e8def51fe3f8cce855ea54f5f19
In case failed of command "realtime show pgsql status" show a message the data
of connection to more clear information in error.
Change-Id: Ia8e9e2400466606e7118f52a46e05df0719b6a29
Device state subscription lifetimes were governed by when the
subscription was established and unsubscribed from. However, it is
possible that at the time of unsubscription, there could be device state
events still in flight. When those device state events occur, the device
state callback could attempt to dereference a freed pointer. Crash.
This change ensures that the lifetime of the device state subscription
does not end until the underlying stasis subscription has confirmed that
its final message has been sent.
Change-Id: I25a0f1472894c1a562252fb7129671478e25e9b2
During ICE negotiation the IPs of the local interfaces are sent to the remote
peer as host candidates. In many cases Asterisk is behind a static one-to-one
NAT, so these host addresses will be internal IP addresses.
To help in hiding the topology of the internal network, this patch adds the
ability to override the host candidates by matching them against a
user-defined list of replacements.
Change-Id: I1c9541af97b83a4c690c8150d19bf7202c8bff1f
test_dlinklists doesn't need to NOTICE everyone that every macro worked.
res_phoneprov doesn't need to VERBOSE everyone that a phoneprov extension or
provider was registered.
res_odbc was missing a newline at the end of one message.
Change-Id: I6c06361518ef3711821795e535acd439782a995e
A regression was introduced where searching for realtime PJSIP objects
by regex by starting the regex with a leading "^" would cause no items
to be returned.
This was due to a change which attempted to drop the requirement for a
leading "^" to be present due to how some CLI commands formulate their
regexes. However, the change, rather than simply eliminating the
requirement, caused any regexes that did begin with "^" to end up not
returning the expected results.
This change fixes the problem by inspecting the regex and formulating
the realtime query differently depending on if it begins with "^".
ASTERISK-25702 #close
Reported by Nic Colledge
Patches:
realtime_retrieve_regex.patch submitted by Alexei Gradinari License #5691
Change-Id: I055df608a6e6a10732044fa737a9fe8dca602693
The module res_xmpp does not accept usernames in the form used in component
mode (XEP-0114). In component mode there is no @something in the name.
In component mode the connection is now not dropped anymore.
If the xmpp server sends out a "stream" tag before handshake is finished,
the connection gets dropped in res_xmpp. Now this tag will be ignored and
the connection will be established.
After connecting there will be an exchange of presence states. This does
not work as expected in component mode. The responsible function
"xmpp_pak_presence" is left before the states get sent out. Sending
presence states in component mode is now moved to the top of the function.
ASTERISK-25735 #close
Change-Id: I70e036f931c3124ebb2ad1e56f93ed35cfdd9d5c
A recent change to queue channel variable setting to the Stasis control
queue caused a regression. When setting channel variables, it is
possible to give a NULL channel variable value in order to unset the
variable (i.e. remove it from the channel variable list). The change
introduced a call to ast_variable_new(), which is not tolerant of NULL
channel variable values.
This new change switches from using ast_variable to using a custom
channel variable struct that is lighter weight and NULL value-tolerant.
Change-Id: I784d7beaaa3c036ea936d103e7caf0bb1562162d
A test recently uncovered that running an ill-timed AMI command to show
inbound subscriptions could cause a crash since Asterisk will try to
operate on a freed subscription.
The fix for this is to remove the subscription tree from the list of
subscriptions at the time that we are sending our final NOTIFY request
out. This way, as the subscription is in the process of dying, it is
inaccessible from AMI.
Change-Id: Ic0239003d8d73e04c47c12dd2a7e23867e5b5b23
When queuing tasks onto the Stasis control queue, you can pass an
arbitrary data pointer and a function to free that data. All ARI
commands that use the Stasis control queue made the assumption that the
destructor function would be called in all paths, whether the task was
queued successfully or not. However, this was not correct. If a task was
queued onto a control structure that was already completed, the
allocated data would not be freed properly.
This patch corrects this by making sure that all return paths call the
data destructor.
Change-Id: Ibf06522094f8e5c4cce652537dc5d7222b1c4fcb
A crash occurred when attempting to set a channel variable on a channel
that had already been hung up. This is because there is a small window
between when a control is grabbed and when the channel variable is set
that the channel can be hung up.
The fix here is to queue the setting of the channel variable onto the
control queue. This way, the manipulation of the channel happens in a
thread where it is safe to be done.
In this change, I also noticed that the setting of bridge roles on
channels was being done outside of the control queue, so I also changed
those operations to be done in the control queue.
ASTERISK-25709 #close
Reported by Mark Michelson
Change-Id: I2a0a4d51bce6fba6f1d9954e40935e42f366ea78
Asterisk by default will create a single database connection and share
it among all threads that attempt to access the database. In previous
versions of Asterisk, this was tolerable, because the most used channel
driver, chan_sip, mostly accessed the database from a single thread.
With PJSIP, however, many threads may be attempting to perform database
operations, and there is the potential for many more database accesses,
meaning the concurrency is a horrible bottleneck if only one connection
is shared.
Asterisk has a connection pooling facility built into it, but the
implementation has flaws. For one, there is a strict limit on the number
of simultaneous connections that could be made to the database. Anything
beyond the maximum would result in a failed operation. Attempting to
predict what the maximum should be is nearly impossible even for someone
intimately familiar with Asterisk's threading model. In addition, use of
transactions in the dialplan can cause some severe bugs if connection
pooling is enabled.
This commit seeks to fix the concurrency problem by removing all
connection management code from Asterisk and leaving that to the
underlying unixODBC code instead. Now, Asterisk does not share a single
connection, nor does it try to maintain a connection pool. Instead, all
Asterisk ever does is request a connection from unixODBC and allow
unixODBC to either allocate those connections or retrieve them from a
pool.
Doing this has a bit of a ripple effect. For one, since connections are
not long-lived objects, several of the safeguards that previously
existed have been removed. We don't have to worry about trying to use a
connection that has gone stale. In every case, when we request a
connection, it has just been made and we don't need to perform any
sanity checks to be sure it's still active.
Another major player affected by this change is transactions.
Transactions and their respective connections were so tightly coupled
that it was almost pornographic. This code change moves
transaction-related code to its own file separate from the core ODBC
functionality. This way, the core of ODBC does not even have to know
that transactions exist.
In making this large change, I had to look at a lot of code and
understand it. When making this change, I discovered several places
where the behavior is definitely not ideal, but it seemed outside the
scope of this change to be fixing it. Instead, any place where I saw
some sort of room for improvement has had a XXX comment added explaining
what could be altered to improve it.
Change-Id: I37a84def5ea4ddf93868ce8105f39de078297fbf
Dump the res_pjsip endpt internals.
In non-developer mode we will not document or make easily accessible the
"details" option even though it is still available. The user has to know
it exists to use it. Presumably they would also be aware of the potential
crash warning below.
Warning: PJPROJECT documents that the function used by this CLI command
may cause a crash when asking for details because it tries to access all
active memory pools.
Change-Id: If2d98a3641c9873364d1daaad971376311aef3cb
res_pjsip_log_forwarder has been renamed to res_pjproject
and enhanced as follows:
As a follow-on to the recent 'Add CLI "pjsip show buildopts"' patch,
a new ast_pjproject_get_buildopt function has been added. It
allows the caller to get the value of one of the buildopts.
The initial use case is retrieving the runtime value of
PJ_MAX_HOSTNAME to insure we don't send a hostname greater
than pjproject can handle. Since it can differ between
the version of pjproject that Asterisk was compiled against
and the version of pjproject that Asterisk is running against,
we can't use the PJ_MAX_HOSTNAME macro directly in Asterisk
source code.
Change-Id: Iab6e82fec3d7cf00c1cf6185c42be3e7569dee1e
Added new global option (regcontext) to pjsip. When set, Asterisk will
dynamically create and destroy a NoOp priority 1 extension
for a given endpoint who registers or unregisters with us.
ASTERISK-25670 #close
Reported-by: Daniel Journo
Change-Id: Ib1530c5b45340625805c057f8ff1fb240a43ea62
There are two ways in which the reload() function in res_musiconhold can be
called from the CLI:
* module reload res_musiconhold.so
* moh reload
In the former case, the module loader holds a lock that prevents multiple
concurrent calls, but in the latter there is no such protection.
This patch changes the 'moh reload' CLI command to invoke the module loader
directly, rather than call reload() explicitly.
ASTERISK-25687 #close
Change-Id: I408968b4c8932864411b7f9ad88cfdc7b9ba711c
PJPROJECT has a function available to dump the compile time
options used when building the library.
* Add CLI "pjsip show buildopts" command.
* Update contrib/scripts/autosupport to get pjproject information.
Change-Id: Id93a6a916d765b2a2e5a1aeb54caaf83206be748
res_sorcery_realtime's search-by-regex callback performed a check to
ensure that the passed-in regex began with a caret (^). If it did not,
then no results would be returned.
This callback only started to become used when "like" support was added
to PJSIP CLI commands. The CLI command for listing objects would pass an
empty regex ("") to the sorcery backend if no "like" statement was
present. For most sorcery backends, this resulted in returning all
objects. However, for realtime, this resulted in returning no objects.
This commit seeks to fix the regression by removing the requirement from
res_sorcery_realtime for the passed-in-regex to begin with a caret.
ASTERISK-25689 #close
Reported by Marcelo Terres
Change-Id: I22b4dc5d7f3f11bb29ac2e42ef94682e9bab3b20
On a system with multiple ip addresses in the same subnet, if a
transport is bound to a specific ip address and endpoint/media_address
is set, the SIP/SDP will have the correct address in all fields but
the rtp stream MAY still originate from one of the other ip addresses,
most probably the "primary" ip address. This happens because
res_pjsip_sdp_rtp/create_rtp always calls ast_instance_new with
the "all" ip address (0.0.0.0 or ::).
The new option causes res_pjsip_sdp_rtp/create_rtp to call
ast_rtp_instance_new with the endpoint's media_address (if specified)
instead of the "all" address. This causes the packets to originate from
the specified address.
ASTERISK-25632
ASTERISK-25637
Reported-by: Olivier Krief
Reported-by: Dan Journo
Change-Id: I3dfaa079e54ba7fb7c4fd1f5f7bd9509bbf8bd88
Due to locking issues within pjnath these changes are being
reverted until pjnath can be changed.
ASTERISK-25645
Revert "res_rtp_asterisk.c: Fix DTLS negotiation delays."
This reverts commit 24ae124e4f.
Change-Id: I2986cfb2c43dc14455c1bcaf92c3804f9da49705
Revert "res_rtp_asterisk: Resolve further timing issues with DTLS negotiation"
This reverts commit 965a0eee46.
Change-Id: Ie68fafde27dad4b03cb7a1e27ce2a8502c3f7bbe
This reverts commit 0a9941de9d.
Matt,
This patch causes another problem and should not have been needed.
Before this patch, persistent_endpoint_contact_deleted_observer WAS
deleting the contact_status when ast_sip_location_delete_contact was
called. By deleting it yourself in ast_sip_location_delete_contact
it was gone before the observer could run and the observer therefore
was throwing an error and not sending stasis/AMI/statsd messages.
So, I don't think this was the cause of your original issue. I also
had verified the contact AMI and statsd lifecycle and it was working.
I'll double check now though.
ASTERISK-25675
Reported-by: Daniel Journo
Change-Id: Ib586a6b7f90acb641b0c410f659743ab90e84f1a
This change causes res_crypto to unregister CLI at shutdown while still
preventing the module from being unloaded.
ASTERISK-25673 #close
Change-Id: Ie5d57338dc2752abfc0dd05d0eec86413f2304fc
