<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/3777/">https://reviewboard.asterisk.org/r/3777/</a>
</td>
</tr>
</table>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/3777/diff/2/?file=64886#file64886line1001" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/main/loader.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static enum ast_module_load_result start_resource(struct ast_module *mod)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">999</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="cm">/* Make sure the newly started module is at the end of the list */</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1000</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">AST_DLLIST_LOCK</span><span class="p">(</span><span class="o">&</span><span class="n">module_list</span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1001</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">AST_DLLIST_REMOVE</span><span class="p">(</span><span class="o">&</span><span class="n">module_list</span><span class="p">,</span> <span class="n">mod</span><span class="p">,</span> <span class="n">entry</span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1002</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">AST_DLLIST_INSERT_TAIL</span><span class="p">(</span><span class="o">&</span><span class="n">module_list</span><span class="p">,</span> <span class="n">mod</span><span class="p">,</span> <span class="n">entry</span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1003</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">AST_DLLIST_UNLOCK</span><span class="p">(</span><span class="o">&</span><span class="n">module_list</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Feel free to cringe at this suggestion, but since you've created both an AST_DLLIST_ENTRY and an AST_LIST_ENTRY on the ast_module structure, you could maintain two parallel lists of modules. You'd have the module_list, that is important for module loading and unloading, and you could have the alphabetical list that is used by the CLI command.
At the time that you add the module to the module_list (what I've highlighted), you can also add it to the alphabetical list. That way, you would not have to lock the module_list and rebuild the alphabetical list each time ast_update_module_list() is called. Up to you really.</pre>
</div>
<br />
<p>- Mark Michelson</p>
<br />
<p>On July 21st, 2014, 4:58 p.m. UTC, Matt Jordan wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.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>
<p style="color: grey;"><i>Updated July 21, 2014, 4:58 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</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;">When Asterisk starts a module (calling its load_module function), it re-orders the module list, sorting it alphabetically. Ostensibly, this was done so that the output of 'module show' listed modules in alphabetic order. This had the unfortunate side effect of making modules with complex usage patterns practically unloadable. A module that has a large number of modules that depend on it is typically abandoned during the unloading process. This results in its memory not being reclaimed during exit.
Generally, this isn't harmful - when the process is destroyed, the operating system will reclaim all memory allocated by the process. However, it makes tracking memory leaks or ref debug leaks a real pain.
While this patch is not a complete overhaul of the module loader - such an effort would be beyond the scope of what could be done for Asterisk 13 - this does make some marginal improvements to the loader such that modules like res_pjsip or res_stasis *may* be made properly un-loadable in the future.
1. The linked list of modules has been replaced with a doubly linked list. This allows traversal of the module list to occur backwards. The module shutdown routine now walks the global list backwards when it attempts to unload modules.
2. The alphabetic reorganization of the module list on startup has been removed. Instead, a started module is placed at the end of the module list.
3. The ast_update_module_list function - which is used by the CLI to display the modules - now does the sorting alphabetically itself. It creates its own linked list and inserts the modules into it in alphabetic order. This allows for the intent of the previous code to be maintained.
This patch also contains a fix for res_calendar. Without calendar.conf, the calendar modules were improperly bumping the use count of res_calendar, then failing to load themselves. This patch makes it so that we detect whether or not calendaring is enabled before altering the use count.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;">Verified that the CLI command worked appropriately.
Verified that module loading/unloading of res_calendar (whose calendar modules modify the res_calendar use count) loaded/unloaded properly:
Asterisk Dynamic Loader Starting:
[Jul 14 15:14:52] NOTICE[11877]: loader.c:1317 load_modules: 6 modules will be loaded.
Loading res_calendar.so.
Loading res_calendar_icalendar.so.
Loading res_calendar_exchange.so.
Loading res_calendar_caldav.so.
Loading res_calendar_ews.so.
Asterisk Ready.
*CLI> core stop gracefully
Waiting for inactivity to perform halt...
Unloading res_calendar_ews.so
Unloading res_calendar_caldav.so
Unloading res_calendar_exchange.so
Unloading res_calendar_icalendar.so
Unloading res_calendar.so
Asterisk cleanly ending (0).
Executing last minute cleanups
Verified that using autoload with all modules, module are started as they were previously, and now are stopped/unloaded in the reverse order.</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>/trunk/res/res_calendar.c <span style="color: grey">(419109)</span></li>
<li>/trunk/main/loader.c <span style="color: grey">(419109)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3777/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>