<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&#39;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&#39;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&#39;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>





 <p>On February 21st, 2012, 10:14 a.m., <b>wdoekes</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;">&gt; But who does the free&#39;ing of mod then?

Ok, I was a little quick there. I didn&#39;t see that there are two places where destruction of mod is legit.

&gt; 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&#39;t figure out why:

----

[modules]
load =&gt; func_strings.so
load =&gt; res_calendar.so
load =&gt; res_calendar_caldav.so
load =&gt; res_calendar_ews.so

*CLI&gt; 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

&gt;&gt;&gt; attempting unload of res_calendar.so (0/0)
&gt;&gt;&gt; attempting unload of func_strings.so (0/0)
(no crash)

----

[modules]
load =&gt; res_calendar.so
load =&gt; res_calendar_caldav.so
load =&gt; res_calendar_ews.so
load =&gt; func_strings.so

*CLI&gt; 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

&gt;&gt;&gt; attempting unload of res_calendar.so (0/0)
&gt;&gt;&gt; attempting unload of res_calendar_caldav.so (0/0)
loader.c line 503 (ast_module_shutdown): Error: attempt to destroy invalid mutex &#39;&amp;(&amp;mod-&gt;users)-&gt;lock&#39;.
WARNING: Freeing unused memory at 0x7f37506f1738, in ast_module_shutdown of loader.c, line 504
(CRASH)

----

If I use your patch, I get this:

&gt;&gt;&gt; attempting unload of res_calendar.so (0/0)
&gt;&gt;&gt; attempting unload of res_calendar_caldav.so (0/0)
(no crash)

That was a listing of mod-&gt;resource. If it worked like it&#39;s supposed to
I&#39;d expect to see &quot;func_strings.so&quot;, not &quot;res_calendar_caldav.so&quot;.</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;">Yeah, in 10+ it crashes because we sort the module list. It only seems to crash when a calendar tech module immediately follows res_calendar.so. If you put one module in between them, no crash. I&#39;m assuming it has to do with an interaction with the list macros and -&gt;next disappearing. I&#39;ll keep looking.</pre>
<br />








<p>- Terry</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 &#39;core stop gracefully&#39;, 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 &#39;core stop gracefully&#39;.
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>