<div dir="ltr">Apologies, committed the wrong code. I sort out now<br></div><br><div class="gmail_quote">On Wed, 13 May 2015 at 08:59 Daniel-Constantin Mierla <<a href="mailto:miconda@gmail.com">miconda@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
Hi Jason,<br>
<br>
the commit doesn't seem correct, because looks like you removed the
locking completely. It should still stay with locking on reply
mutex.<br>
<br>
Can you revert the last commit? Then I can take care of removing the
unnecessary code -- note that the code before your commit was
already using the reply lock, so was like it should just be, with
dedicated mutex code being inside defines which were not enabled.<br>
<br>
Cheers,<br>
Daniel</div><div bgcolor="#FFFFFF" text="#000000"><br>
<br>
<div>On 13/05/15 08:56, Jason Penton wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hey Daniel,<br>
<br>
<div>You are correct this is no longer needed - we actually only
used this to be able to set a flag on the transaction that we
later used for branch picking. We made a fix a while back for
doing the correct branch picking which no longer required this
extra flag and therefore no longer required the mutex. I have
tested in our env and all looks good.</div>
<div><br>
</div>
<div>p.s. I have committed the cleanup.</div>
<div><br>
</div>
<div>Cheers</div>
<div>Jason</div>
</div>
<br>
<div class="gmail_quote">On Tue, 12 May 2015 at 10:34
Daniel-Constantin Mierla <<a href="mailto:miconda@gmail.com" target="_blank">miconda@gmail.com</a>>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"> Hi Jason,<br>
<br>
ok, would be great to sort it out properly, thanks for your
time!<br>
<br>
Cheers,<br>
Daniel</div>
<div bgcolor="#FFFFFF" text="#000000"><br>
<br>
<div>On 12/05/15 10:10, Jason Penton wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hey Daniel,<br>
<br>
Okay great, let me look into this. It will be great if
we have one less mutex to worry about ;) - If not
required I will remove and commit.
<div><br>
</div>
<div>Cheers</div>
<div>Jason</div>
</div>
<br>
<div class="gmail_quote">On Mon, 11 May 2015 at 09:55
Daniel-Constantin Mierla <<a href="mailto:miconda@gmail.com" target="_blank">miconda@gmail.com</a>>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi
Jason,<br>
<br>
over the weekend I pushed a patch that disables the
use of dedicated<br>
mutex for t_continue(). It can be enabled by defining
ENABLE_ASYNC_MUTEX.<br>
<br>
While investigated some reports of crash when removing
from time, I<br>
found the potential of a race when t_coninue() is
executed at the time<br>
the fr_timer for suspended transaction elapsed. The
timer process will<br>
get the transaction out and remove it from timer under
the reply lock<br>
and the worker doing t_continue() will get it out
under the async lock.<br>
<br>
I looked at the commit you did when introducing the
dedicated async<br>
mutex, the note being:<br>
<br>
- "dedicated lock to prevent multiple
invocations of suspend on<br>
tz (reply lock used to be used)"<br>
<br>
Perhaps tz is tx and stands for transmission -
however, the reply lock<br>
should be safe for this case as well. Moreover, the
continue is like the<br>
suspended branch got a reply and transaction continues
processing, which<br>
implies the reply lock is aquired (like execution of
failure_route,<br>
which can also happen if fr_timer elapses before
t_continue() is executed).<br>
<br>
Given those, I don't see anymore a reason for
dedicated async mutex.<br>
Also, it protects to races of using two mutexes, which
can easily lead<br>
to deadlocks (e.g., one process acquires the reply
lock and tries to get<br>
the async lock while another one wanted first the
reply lock and later<br>
the async lock).<br>
<br>
For now I disabled the code with defines, as I wanted
to discuss and be<br>
sure I haven't overlooked any issue you tried to avoid
with the<br>
dedicated mutex. Let me know what you think about.<br>
<br>
Cheers,<br>
Daniel<br>
<br>
--<br>
Daniel-Constantin Mierla<br>
<a href="http://twitter.com/#%21/miconda" target="_blank">http://twitter.com/#!/miconda</a> -
<a href="http://www.linkedin.com/in/miconda" target="_blank">http://www.linkedin.com/in/miconda</a><br>
Kamailio World Conference, May 27-29, 2015<br>
Berlin, Germany - <a href="http://www.kamailioworld.com" target="_blank">http://www.kamailioworld.com</a><br>
<br>
</blockquote>
</div>
</blockquote>
<br>
<pre cols="72">--
Daniel-Constantin Mierla
<a href="http://twitter.com/#%21/miconda" target="_blank">http://twitter.com/#!/miconda</a> - <a href="http://www.linkedin.com/in/miconda" target="_blank">http://www.linkedin.com/in/miconda</a>
Kamailio World Conference, May 27-29, 2015
Berlin, Germany - <a href="http://www.kamailioworld.com" target="_blank">http://www.kamailioworld.com</a></pre>
</div>
</blockquote>
</div>
</blockquote>
<br>
<pre cols="72">--
Daniel-Constantin Mierla
<a href="http://twitter.com/#!/miconda" target="_blank">http://twitter.com/#!/miconda</a> - <a href="http://www.linkedin.com/in/miconda" target="_blank">http://www.linkedin.com/in/miconda</a>
Kamailio World Conference, May 27-29, 2015
Berlin, Germany - <a href="http://www.kamailioworld.com" target="_blank">http://www.kamailioworld.com</a></pre>
</div></blockquote></div>