| Age | Commit message (Collapse) | Author |
|---|
|
This is a type mismatch fix of the debugging commit
c63316eec10e1990a88bf4712238d6deb375bfa9 made to find out why
a testsuite test was failing only on one of the continuous
integration build agents.
Change-Id: Iba34f6e87cec331f6ac80e4daff6476ea6f00a75
|
|
Commit 39cc28f6ea2140ad6d561fd4c9e9a66f065cecee attempted to fix a
test failure observed on 32 bit test agents by ensuring that a cast from
a 32 bit unsigned integer to a 64 bit unsigned integer was happening in
a predictable place. As it turns out, this did not cause test runs to
succeed.
This commit adds several redundant debug messages that print the payload
lengths of websocket frames. The idea here is that this commit will not
cause tests to succeed for the faulty test agent, but we might deduce
where the fault lies more easily this way by observing at what point the
expected value (537) changes to some ungangly huge number.
If you are wondering why something like this is being committed to the
branch, keep in mind that in commit
39cc28f6ea2140ad6d561fd4c9e9a66f065cecee I noted that the observed test
failures only happen when automated tests are run. Attempts to run the
tests by hand manually on the test agent result in the tests passing.
Change-Id: I14a65c19d8af40dadcdbd52348de3b0016e1ae8d
|
|
We have seen a rash of test failures on a 32-bit build agent. Commit
48698a5e21d7307f61b5fb2bd39fd593bc1423ca solved an obvious problem where
we were not encoding a 64-bit value correctly over the wire. This
commit, however, did not solve the test failures.
In the failing tests, ARI is attempting to send a 537 byte text frame
over a websocket. When sending a frame this small, 16 bits are all that
is required in order to encode the payload length on the websocket
frame. However, ast_websocket_write() thinks that the payload length is
greater than 65535 and therefore writes out a 64 bit payload length.
Inspecting this payload length, the lower 32 bits are exactly what we
would expect it to be, 537 in hex. The upper 32 bits, are junk values
that are not expected to be there.
In the failure, we are passing the result of strlen() to a function that
expects a uint64_t parameter to be passed in. strlen() returns a size_t,
which on this 32-bit machine is 32 bits wide. Normally, passing a 32-bit
unsigned value to somewhere where a 64-bit unsigned value is expected
would cause no problems. In fact, in manual runs of failing tests, this
works just fine. However, ast_websocket_write() uses the Asterisk
optional API, which means that rather than a simple function call, there
are a series of macros that are used for its declaration and
implementation. These macros may be causing some sort of error to occur
when converting from a 32 bit quantity to a 64 bit quantity.
This commit changes the logic by making existing ast_websocket_write()
calls use ast_websocket_write_string() instead. Within
ast_websocket_write_string(), the 64-bit converted strlen is saved in a
local variable, and that variable is passed to ast_websocket_write()
instead.
Note that this commit message is full of speculation rather than
certainty. This is because the observed test failures, while always
present in automated test runs, never occur when tests are manually
attempted on the same test agent. The idea behind this commit is to fix
a theoretical issue by performing changes that should, at the least,
cause no harm. If it turns out that this change does not fix the failing
tests, then this commit should be reverted.
Change-Id: I4458dd87d785ca322b89c152b223a540a3d23e67
|
|
Prior to ASTERISK-24988, the WebSocket handshake was resolved before Stasis
applications were registered. This was done such that the WebSocket would be
ready when an application is registered. However, by creating the WebSocket
first, the client had the ability to make requests for the Stasis application
it thought had been created with the initial handshake request. The inevitable
conclusion of this scenario was the cart being put before the horse.
ASTERISK-24988 resolved half of the problem by ensuring that the applications
were created and registered with Stasis prior to completing the handshake
with the client. While this meant that Stasis was ready when the client
received the green-light from Asterisk, it also meant that the WebSocket was
not yet ready for Stasis to dispatch messages.
This patch introduces a message queuing mechanism for delaying messages from
Stasis applications while the WebSocket is being constructed. When the ARI
event processor receives the message from the WebSocket that it is being
created, the event processor instantiates an event session which contains a
message queue. It then tries to create and register the requested applications
with Stasis. Messages that are dispatched from Stasis between this point and
the point at which the event processor is notified the WebSocket is ready, are
stashed in the queue. Once the WebSocket has been built, the queue's messages
are dispatched in the order in which they were originally received and the
queue is concurrently cleared.
ASTERISK-25181 #close
Reported By: Matt Jordan
Change-Id: Iafef7b85a2e0bf78c114db4c87ffc3d16d671a17
|
|
Git does not support the ability to replace a token with a version
string during check-in. While it does have support for replacing a
token on clone, this is somewhat sub-optimal: the token is replaced
with the object hash, which is not particularly easy for human
consumption. What's more, in practice, the source file version was often
not terribly useful. Generally, when triaging bugs, the overall version
of Asterisk is far more useful than an individual SVN version of a file. As a
result, this patch removes Asterisk's support for showing source file
versions.
Specifically, it does the following:
* Rename ASTERISK_FILE_VERSION macro to ASTERISK_REGISTER_FILE, and
remove passing the version in with the macro. Other facilities
than 'core show file version' make use of the file names, such as
setting a debug level only on a specific file. As such, the act of
registering source files with the Asterisk core still has use. The
macro rename now reflects the new macro purpose.
* main/asterisk:
- Refactor the file_version structure to reflect that it no longer
tracks a version field.
- Remove the "core show file version" CLI command. Without the file
version, it is no longer useful.
- Remove the ast_file_version_find function. The file version is no
longer tracked.
- Rename ast_register_file_version/ast_unregister_file_version to
ast_register_file/ast_unregister_file, respectively.
* main/manager: Remove value from the Version key of the ModuleCheck
Action. The actual key itself has not been removed, as doing so would
absolutely constitute a backwards incompatible change. However, since
the file version is no longer tracked, there is no need to attempt to
include it in the Version key.
* UPGRADE: Add notes for:
- Modification to the ModuleCheck AMI Action
- Removal of the "core show file version" CLI command
Change-Id: I6cf0ff280e1668bf4957dc21f32a5ff43444a40e
|
|
When merging the websocket timeout issue (ASTERISK-24701) an extra, almost
duplicate, check was left in the code that should not have been. This removes
it.
ASTERISK-24701 #close
Reported by: Matt Jordan
Review: https://reviewboard.asterisk.org/r/4412/
........
Merged revisions 431693 from http://svn.asterisk.org/svn/asterisk/branches/13
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@431695 65c4cc65-6c06-0410-ace0-fbb531ad65f3
|
|
When writing to a websocket if a timeout occurred the underlying socket did not
get closed/disconnected. This patch makes sure the websocket gets disconnected
on a write timeout. Also a notice is logged stating that the websocket was
disconnected.
ASTERISK-24701 #close
Reported by: Matt Jordan
Review: https://reviewboard.asterisk.org/r/4412/
........
Merged revisions 431669 from http://svn.asterisk.org/svn/asterisk/branches/11
........
Merged revisions 431670 from http://svn.asterisk.org/svn/asterisk/branches/13
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@431671 65c4cc65-6c06-0410-ace0-fbb531ad65f3
|
|
When a client takes a long time to process information received from Asterisk,
a write operation using fwrite may fail to write all information. This causes
the underlying file stream to be in an unknown state, such that the socket
must be disconnected. Unfortunately, there are two problems with this in
Asterisk's existing websocket code:
1. Periodically, during the read loop, Asterisk must write to the connected
websocket to respond to pings. As such, Asterisk maintains a reference to
the session during the loop. When ast_http_websocket_write fails, it may
cause the session to decrement its ref count, but this in and of itself
does not break the read loop. The read loop's write, on the other hand,
does not break the loop if it fails. This causes the socket to get in a
'stuck' state, preventing the client from reconnecting to the server.
2. More importantly, however, is that the fwrite in ast_http_websocket_write
fails with a large volume of data when the client takes awhile to process
the information. When it does fail, it fails writing only a portion of
the bytes. With some debugging, it was shown that this was failing in a
similar fashion to ASTERISK-12767. Switching this over to ast_careful_fwrite
with a long enough timeout solved the problem.
Note that this version of the patch, unlike r417310 in Asterisk 11, exposes
configuration options beyond just chan_sip's sip.conf. Configuration options
to configure the write timeout have also been added to pjsip.conf and ari.conf.
#ASTERISK-23917 #close
Reported by: Matt Jordan
Review: https://reviewboard.asterisk.org/r/3624/
........
Merged revisions 417310 from http://svn.asterisk.org/svn/asterisk/branches/11
........
Merged revisions 417311 from http://svn.asterisk.org/svn/asterisk/branches/12
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@417317 65c4cc65-6c06-0410-ace0-fbb531ad65f3
|
|
This adds DEBUG level logging for ARI websocket events and HTTP
responses similar to what is available for AMI. Logging for ARI HTTP
requests is already adequate for debugging purposes.
........
Merged revisions 412565 from http://svn.asterisk.org/svn/asterisk/branches/12
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@412566 65c4cc65-6c06-0410-ace0-fbb531ad65f3
|
|
With the new work in Asterisk 12, there are some uses of the
optional_api that are prone to failure. The details are rather involved,
and captured on [the wiki][1].
This patch addresses the issue by removing almost all of the magic from
the optional API implementation. Instead of relying on weak symbol
resolution, a new optional_api.c module was added to Asterisk core.
For modules providing an optional API, the pointer to the implementation
function is registered with the core. For modules that use an optional
API, a pointer to a stub function, along with a optional_ref function
pointer are registered with the core. The optional_ref function pointers
is set to the implementation function when it's provided, or the stub
function when it's now.
Since the implementation no longer relies on magic, it is now supported
on all platforms. In the spirit of choice, an OPTIONAL_API flag was
added, so we can disable the optional_api if needed (maybe it's buggy on
some bizarre platform I haven't tested on)
The AST_OPTIONAL_API*() macros themselves remained unchanged, so
existing code could remain unchanged. But to help with debugging the
optional_api, the patch limits the #include of optional API's to just
the modules using the API. This also reduces resource waste maintaining
optional_ref pointers that aren't used.
Other changes made as a part of this patch:
* The stubs for http_websocket that wrap system calls set errno to
ENOSYS.
* res_http_websocket now properly increments module use count.
* In loader.c, the while() wrappers around dlclose() were removed. The
while(!dlclose()) is actually an anti-pattern, which can lead to
infinite loops if the module you're attempting to unload exports a
symbol that was directly linked to.
* The special handling of nonoptreq on systems without weak symbol
support was removed, since we no longer rely on weak symbols for
optional_api.
[1]: https://wiki.asterisk.org/wiki/x/wACUAQ
(closes issue ASTERISK-22296)
Reported by: Matt Jordan
Review: https://reviewboard.asterisk.org/r/2797/
........
Merged revisions 397989 from http://svn.asterisk.org/svn/asterisk/branches/12
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@397990 65c4cc65-6c06-0410-ace0-fbb531ad65f3
|
|
In tracking down some unit tests failures, I ended up reading the fine
print[1] regarding Jansson's thread safety.
In short:
1. Ref-counting is non-atomic.
2. json_dumps() and friends are not thread safe.
This patch adds locking where necessary to our ast_json_* wrapper API,
with documentation in json.h describing the thread safety limitations of
the API.
[1]: http://www.digip.org/jansson/doc/2.4/portability.html#thread-safety
Review: https://reviewboard.asterisk.org/r/2716/
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@396119 65c4cc65-6c06-0410-ace0-fbb531ad65f3
|
|
This renames all files and API calls from several variants of
Stasis-HTTP to ARI including:
* Stasis-HTTP -> ARI
* STASIS_HTTP -> ARI
* stasis_http -> ari (ast_ari for global symbols, file names as well)
* stasis http -> ARI
Review: https://reviewboard.asterisk.org/r/2706/
(closes issue ASTERISK-22136)
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@395603 65c4cc65-6c06-0410-ace0-fbb531ad65f3
|
