<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/1752/">https://reviewboard.asterisk.org/r/1752/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 20th, 2012, 2:33 p.m., <b>Mark Michelson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think a better idea is to call both ast_unload_resource() and ast_module_unregister() in res_calendar.c.</pre>
</blockquote>
<p>On February 20th, 2012, 2:38 p.m., <b>Terry Wilson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'd have to pass around pointers to the ast_module_info instead of just using AST_MODULE. The weird thing is that 1.8 doesn't seem to have the problem at all. Something has changed between there and trunk that is screwing things up.</pre>
</blockquote>
<p>On February 21st, 2012, 3:48 a.m., <b>Tilghman Lesher</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The module shutdown routine has a loop that permits dependency ordering, if the usecount is set greater than zero. It may be helpful to increment the usecount of res_calendar for each dependent module registered. That would allow you to delay the unload of res_calendar until after all dependent modules are unloaded. There's no good way of doing this currently, however, as usecount is defined within loader.c, and it is not accessible without providing a channel for sending a softhangup to at the appropriate moment.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> But who does the free'ing of mod then?
Ok, I was a little quick there. I didn't see that there are two places where destruction of mod is legit.
> Something has changed between there and trunk that is screwing things up.
That it crashes in 10 has everything to do with the load order -- the new sorting -- but I can't figure out why:
----
[modules]
load => func_strings.so
load => res_calendar.so
load => res_calendar_caldav.so
load => res_calendar_ews.so
*CLI> module show
Module Description Use Count
res_calendar.so Asterisk Calendar integration 0
func_strings.so String handling dialplan functions 0
res_calendar_caldav.so Asterisk CalDAV Calendar Integration 0
res_calendar_ews.so Asterisk MS Exchange Web Service Calenda 0
4 modules loaded
>>> attempting unload of res_calendar.so (0/0)
>>> attempting unload of func_strings.so (0/0)
(no crash)
----
[modules]
load => res_calendar.so
load => res_calendar_caldav.so
load => res_calendar_ews.so
load => func_strings.so
*CLI> module show
Module Description Use Count
res_calendar.so Asterisk Calendar integration 0
res_calendar_caldav.so Asterisk CalDAV Calendar Integration 0
res_calendar_ews.so Asterisk MS Exchange Web Service Calenda 0
func_strings.so String handling dialplan functions 0
4 modules loaded
>>> attempting unload of res_calendar.so (0/0)
>>> attempting unload of res_calendar_caldav.so (0/0)
loader.c line 503 (ast_module_shutdown): Error: attempt to destroy invalid mutex '&(&mod->users)->lock'.
WARNING: Freeing unused memory at 0x7f37506f1738, in ast_module_shutdown of loader.c, line 504
(CRASH)
----
If I use your patch, I get this:
>>> attempting unload of res_calendar.so (0/0)
>>> attempting unload of res_calendar_caldav.so (0/0)
(no crash)
That was a listing of mod->resource. If it worked like it's supposed to
I'd expect to see "func_strings.so", not "res_calendar_caldav.so".</pre>
<br />
<p>- wdoekes</p>
<br />
<p>On February 18th, 2012, 1:31 p.m., Terry Wilson wrote:</p>
<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 Terry Wilson.</div>
<p style="color: grey;"><i>Updated Feb. 18, 2012, 1:31 p.m.</i></p>
<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;">res_calendar calls ast_unload_resource for the related tech modules when it is unloaded. If this happens through a 'core stop gracefully', then it will be unloading the tech modules that are already in the list that is being traversed (supposedly safely) for unloading, eventually causing a double free. The problem seems to be that ast_unload_resource, while it calls the unload() callback function for the module, does not actually unlink the module from the list of modules. So the AST_LIST_TRAVERSE_SAFE_BEGIN {} still iterates over the unloaded module.
This patch causes ast_unload_resource to call AST_LIST_REMOVE on successfully unloaded modules.</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;">Scenario: Start Asterisk with res_calendar and assorted calendar tech modules loaded. Run 'core stop gracefully'.
Before patch: Crash.
After patch: No crash.</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/main/loader.c <span style="color: grey">(355783)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1752/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>