<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/1821/">https://reviewboard.asterisk.org/r/1821/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Matt Jordan.</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">As part of the Lightweight NAT test development (https://reviewboard.asterisk.org/r/1759/), Josh discovered that the Python subprocess module and twisted do not play well together. At all. Both twisted and the subprocess attempt to handle SIGCHLD signals, with the result that one or the other (typically twisted, however) fail in some fashion or another.
More information on this can be found at the links below:
http://stackoverflow.com/questions/1948641/twisted-threading-with-subprocess-popen
http://armyofevilrobots.com/node/422
http://twistedmatrix.com/trac/ticket/2535
Note that in the case of the Lightweight NAT tests, the behavior was found without even spawning additional processes; the reactor would block indefinitely when told to listen for UDP packets.
To solve this, there were a few approaches that we could take:
1. Abandon twisted. This would also imply that we would have to abandon our usage of starpy for FastAGI/AMI integration. This would not be good, but we could write a non-twisted dependent interface for those two libraries. However, twisted is a robust library that greatly speeds development of communication interfaces beyond FastAGI/AMI. For that reason (and a whole host of others), we decided not to go with this.
2. Instruct twisted to not use sign handlers by passing installSignalHandlers=False to the reactor. While this may make subprocess and twisted play nice with each other, it limits the effectiveness of twisted, and could cause issues elsewhere when third party libraries using twisted make the assumption that the reactor will have signal handlers. This seemed like a workaround, as opposed to a solution.
3. Stop using subprocess and use twisted to manage processes.
We went with option 3. This has the impact of making a lot of previously blocking operations non-blocking (CLI commands, for example) - which while good, does increase the complexity of those operations. At the same time, it has the added benefit of removing a number of polling loops and blocking operations that didn't need to be blocking, with the end result that the tests run much, much faster. (For example: some tests which previously took 15 - 30 seconds now take 2 - 5)
This patch has the following impacts on test development:
1. The Asterisk class now uses non-blocking operations to start/stop itself, and instead returns a deferred object that can be used to be notified when Asterisk has started/stopped. As a result, tests that derive from TestCase now no longer need to call start_asterisk and stop_asterisk, as the starting/stopping of Asterisk is handled by the TestCase class itself. Tests that do not use the TestCase object will need to add callbacks to the deferred object to ensure that Asterisk is fully started/stopped.
2. The Asterisk class's CLI command interfaces are all non-blocking, and return a deferred object that signals when the command has completed.
3. SIPpScenario now returns a deferred when run, which signals when the scenario has ended and its success/failure.
4. A new class, SIPpScenarioSequence has been added, which will run a series of SIPpScenario objects in a synchronous fashion. This eases the burden on people who were using SIPpScenario to run successive tests, as opposed to tests in parallel. (The SIPpTest class now uses this under the hood to run scenarios in a sequence as well)
5. The utils module has been renamed to TestSuiteUtils, to prevent a name conflict with twisted's utils modules.
Note that we still have a number of tests that use pjsua and the subprocess module; as we currently have no library wrapping this usage I've deferred (pun intended) that work to another patch. The Lightweight NAT tests do not use pjsua, and hence are not dependent on that work being finished to be included in the Test Suite.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/asterisk/trunk/tests/mixmonitor/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/mixmonitor_audiohook_inherit/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/udptl_v6/configs/ast1/manager.conf <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/udptl_v6/configs/ast2/manager.conf <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/udptl_v6/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/iax2/basic-call/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/iax2/basic-call/configs/ast1/manager.conf <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/iax2/basic-call/configs/ast2/extensions.conf <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/iax2/basic-call/configs/ast2/manager.conf <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/iax2/basic-call/configs/ast1/extensions.conf <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/feature_blonde_transfer/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/feature_attended_transfer/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/fastagi/say-time/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/fastagi/say-phonetic/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/fastagi/say-digits/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/fastagi/say-number/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/fastagi/say-datetime/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/fastagi/say-date/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/fastagi/say-alpha/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/fastagi/get-data/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/fastagi/hangup/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/dynamic-modules/test-config.yaml <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/dynamic-modules/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/chanspy/chanspy_barge/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/chanspy/chanspy_w_mixmonitor/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_tls_register/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/use_contact_from_200/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/chanspy/chanspy_barge/configs/ast1/manager.conf <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_register_domain_acl/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/realtime_sipregs/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_hold/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/nat_supertest/sipp/inject.csv <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/nat_supertest/test-config.yaml <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/realtime_nosipregs/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/info_dtmf/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/nat_supertest/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/func_vmcount/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/handle_response_address_incomplete/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/apps/incomplete/sip_incomplete/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/version.py <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/runtests.py <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/sippversion.py <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/utils.py <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/cdr.py <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/sipp.py <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/TestConfig.py <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/asterisk.py <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/TestCase.py <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/CDRTestCase.py <span style="color: grey">(3104)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1821/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>