[asterisk-dev] [Code Review] StopMixMonitor race condition (not giving up file immediately)

[Mark Michelson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/283/#review851
-----------------------------------------------------------



/trunk/apps/app_mixmonitor.c
<http://reviewboard.digium.com/r/283/#comment2072>

    You need to hold the mixmonitor->mixmonitor_ds->lock around this section.



/trunk/apps/app_mixmonitor.c
<http://reviewboard.digium.com/r/283/#comment2068>

    These are unnecessary since calloc automatically zeroes all data allocated.



/trunk/apps/app_mixmonitor.c
<http://reviewboard.digium.com/r/283/#comment2069>

    Be sure to destroy the mutex and condition here in addition to freeing the mixmonitor_ds.



/trunk/apps/app_mixmonitor.c
<http://reviewboard.digium.com/r/283/#comment2070>

    Free mixmonitor in this failure case.



/trunk/apps/app_mixmonitor.c
<http://reviewboard.digium.com/r/283/#comment2071>

    This isn't your code, but mixmonitor, mixmonitor->mixmonitor_ds, and all allocated fields inside of mixmonitor_ds need to be freed here.


- Mark


On 2009-06-15 16:12:47, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/283/
> -----------------------------------------------------------
> 
> (Updated 2009-06-15 16:12:47)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> StopMixMonitor only indicates to the MixMonitor thread to stop writing to the file.  It does not guarantee that the recording's file handle is available to the dialplan immediately after execution.  This results in a race condition.
> 
> For Example: After StopMixMonitor, If an AGI script is used to convert a recording to another format, it may or may not have access to the file depending on how fast the MixMonitor thread closes the filestream.
> 
> To resolve this, the filestream pointer is placed in a datastore on the channel. When StopMixMonitor is called, the datastore is retrieved from the channel and the filestream is closed immediately before returning to the dialplan.  Documentation indicating the use of StopMixMonitor to free files has been updated as well.
> 
> 
> This addresses bug 15259.
>     https://issues.asterisk.org/view.php?id=15259
> 
> 
> Diffs
> -----
> 
>   /trunk/doc/datastores.txt 200144 
>   /trunk/apps/app_mixmonitor.c 200144 
> 
> Diff: http://reviewboard.digium.com/r/283/diff
> 
> 
> Testing
> -------
> 
> Tested StopMixMonitor in the dialplan in both a made up extension and the 'h' extension.  Both resulted in the immediate termination of the filestream.
> 
> 
> Thanks,
> 
> David
> 
>