<html><head></head><body><div>Hi,</div><div><br></div><div>The last few weeks have been stupidly busy. That bug (T93) is about having the same lock for sending and receiving.. I'd open a new one about the global lock. The make the lock per agent and pass the agent reference in a weak ref to all of the timeout sources, that should fix the issues we had earlier.</div><div><br></div><div>Olivier</div><div><br></div><div><br></div><div>On Fri, 2016-11-04 at 13:12 +0100, fb111@web.de wrote:</div><blockquote type="cite"><div style="font-family: Verdana;font-size: 12.0px;"><div>
<div> </div>
<div>Hi Olivier,</div>
<div> </div>
<div>have you had the chance to look into this problem? We are also prepared to help, but would appreciate your advise.</div>
<div> </div>
<div>Best,</div>
<div> </div>
<div>Frank</div>
<div> </div>
<div>
<div name="quote" style="margin:10px 5px 5px 10px; padding: 10px 0 10px 10px; border-left:2px solid #C3D9E5; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">
<div style="margin:0 0 10px 0;"><b>Gesendet:</b> Donnerstag, 13. Oktober 2016 um 20:27 Uhr<br>
<b>Von:</b> "Lorenzo Miniero" <lminiero@gmail.com><br>
<b>An:</b> "Olivier Crête" <olivier.crete@collabora.com><br>
<b>Cc:</b> "Anand Sivaram" <aspnair@gmail.com>, "Miguel París Díaz" <mparisdiaz@gmail.com>, nice <nice@lists.freedesktop.org><br>
<b>Betreff:</b> Re: [libnice] Sending packets uses a static, global lock?</div>
<div name="quoted-content">
<div>
<div class="gmail_extra">
<div class="gmail_quote">2016-10-13 16:44 GMT+02:00 Olivier Crête <span><<a href="mailto:olivier.crete@collabora.com" onclick="parent.window.location.href='olivier.crete@collabora.com'; return false;" target="_blank">olivier.crete@collabora.com</a>></span>:
<blockquote type="cite">
<div>
<div>Hi,</div>
<div> </div>
<div>The lock should just be per agent. The global lock was a workaround for the crash that happens when the timeout GSource fires off while the object is being destroyed and then has an invalid agent pointer in the callback. Possibly we just need to use a GWeakRef for the agent in all of the callbacks, possibly by doing a wrapper using agent_timeout_add_with_context(). And then we can also remove the "if (g_source_is_destroyed (g_main_current_source ())) {" everywhere and centralize it all in the wrapper...</div>
<div> </div>
</div>
<br></blockquote>
<div> </div>
<div> </div>
<div>Hi Olivier,</div>
<div> </div>
<div>thanks for the clarification. From what I understood of how sources work in glib, this sounds like a reasonable plan. Do you want me to open an issue somewhere to keep track of these activities? I noticed there seems to be already one that looks related, but I'm not sure what its current state it:</div>
<div> </div>
<div><a href="https://phabricator.freedesktop.org/T93" target="_blank">https://phabricator.freedesktop.org/T93</a></div>
<div> </div>
<div>If there's anything I can do to help, just let me know. I'll be abroad next week for a conference, but when I come back I'm willing to work on the code to help fix this.</div>
<div> </div>
<div>Thanks,</div>
<div>Lorenzo</div>
<div> </div>
<div> </div>
<blockquote type="cite">
<div>
<div> </div>
<div>Olivier</div>
<div>
<div class="gmail-h5">
<div> </div>
<div>On Thu, 2016-10-13 at 11:10 +0200, Lorenzo Miniero wrote:</div>
<blockquote>
<div>
<div class="gmail_extra">
<div class="gmail_quote">2016-10-13 6:46 GMT+02:00 Anand Sivaram <span><<a href="mailto:aspnair@gmail.com" onclick="parent.window.location.href='aspnair@gmail.com'; return false;" target="_blank">aspnair@gmail.com</a>></span>:
<blockquote>
<div>
<div style="font-family: monospace , monospace;font-size: small;">What about having a global flag for the lock type?<br>
Set it with g_object_set during the library initialization.</div>
<div style="font-family: monospace , monospace;font-size: small;">It should have 3 values - Off, Per Agent, Global.</div>
<div style="font-family: monospace , monospace;font-size: small;">Modify agent_lock() and agent_unlock() accordingly.<br>
</div>
<div style="font-family: monospace , monospace;font-size: small;">That way if the applications using libnice implemented their own locking</div>
<div style="font-family: monospace , monospace;font-size: small;">mechanism, that could be used seamlessly.<br>
</div>
</div>
</blockquote>
<div> </div>
<div> </div>
<div>That looks like a smart approach, especially as it would be flexible: the off part is particularly interesting, as I have locks all over the place myself! :-). I'm still concerned about whether or not this can result in issues with the glib2 stuff libnice uses internally, though, and that applications don't have access to? There may be private methods invoked that need that locking, something the application from outside may not be able to protect.</div>
<div> </div>
<div>Lorenzo</div>
<div> </div>
<div> </div>
<div> </div>
<blockquote>
<div>
<div style="font-family: monospace , monospace;font-size: small;"> </div>
<div style="font-family: monospace , monospace;font-size: small;"><span class="gmail-m_4092208918506556890HOEnZb"><font color="#888888">anand</font></span></div>
<div style="font-family: monospace , monospace;font-size: small;"> </div>
<div style="font-family: monospace , monospace;font-size: small;"> </div>
<div style="font-family: monospace , monospace;font-size: small;"> </div>
</div>
<div class="gmail-m_4092208918506556890HOEnZb">
<div class="gmail-m_4092208918506556890h5">
<div class="gmail_extra">
<div class="gmail_quote">On 12 October 2016 at 21:35, Lorenzo Miniero <span><<a href="mailto:lminiero@gmail.com" onclick="parent.window.location.href='lminiero@gmail.com'; return false;" target="_blank">lminiero@gmail.com</a>></span> wrote:
<blockquote>
<div>Thanks for the context! (and sorry for the delay in this response...)
<div> </div>
<div>I'm not familiar enough with glib2, unfortunately: considering the patch dates back to about 7 years ago, is the problem the global muxed fixed still an issue with current versions? We did a quick and dirty test by turning those global agent locks to per-agent locks, where possible (that is, in methods that actually provided a pointer to an agent instance) and we did notice improvements in the performance (about twice as many sessions before it started to break down), but of course this means little if it risks causing those race conditions and crashes again. Is this road worth exploring? I'm willing to help and contribute to such work, of course, although I admit I'm not an expert on the libnice internals (but it's never too late to learn! ;-) ).</div>
<div> </div>
<div>Thanks,</div>
<div>Lorenzo</div>
<div> </div>
</div>
<div class="gmail-m_4092208918506556890m_7447573976023040723HOEnZb">
<div class="gmail-m_4092208918506556890m_7447573976023040723h5">
<div class="gmail_extra">
<div class="gmail_quote">2016-10-05 13:09 GMT+02:00 Miguel París Díaz <span><<a href="mailto:mparisdiaz@gmail.com" onclick="parent.window.location.href='mparisdiaz@gmail.com'; return false;" target="_blank">mparisdiaz@gmail.com</a>></span>:
<blockquote>
<div>Sorry for the copy-paste error.<br>
<br>
<div class="gmail_extra">
<div class="gmail_quote"><span>2016-10-05 13:06 GMT+02:00 Miguel París Díaz <span><<a href="mailto:mparisdiaz@gmail.com" onclick="parent.window.location.href='mparisdiaz@gmail.com'; return false;" target="_blank">mparisdiaz@gmail.com</a>></span>:</span>
<blockquote>
<div>
<div>
<div>
<div><span>Good point Lorenzo!!!</span><br>
</div>
<span>This change was added in this commit:<br>
<br>
commit 2faa76a4840100bebd9532bd47a2755874c5b3a0<br>
Author: Youness Alaoui <<a href="mailto:youness.alaoui@collabora.co.uk" onclick="parent.window.location.href='youness.alaoui@collabora.co.uk'; return false;" target="_blank">youness.alaoui@collabora.co.uk</a>><br>
Date: Tue Jun 16 20:40:<a>33 2009 -0400</a><br>
<br>
Use a global mutex for all nice agents and use g_source_is_destroyed to avoid the race condition of a source being destroyed while we wait for the mutex</span><br>
</div>
<span>And then the RecMutex was changed by a Mutex in:</span></div>
</div>
</blockquote>
<div><br>
commit 1deee69325284c726c3a8380a7d5839a51e20c48<br>
Author: Olivier Crête <<a href="mailto:olivier.crete@collabora.com" onclick="parent.window.location.href='olivier.crete@collabora.com'; return false;" target="_blank">olivier.crete@collabora.com</a>><br>
Date: Mon Feb 24 18:51:31 2014 -0500<br>
<br>
agent: Replace recursive mutex with non-recursive<br>
<br>
This should prevent us from re-adding re-entrancy in the future<br>
</div>
<div>
<div class="gmail-m_4092208918506556890m_7447573976023040723m_-909601598888611618h5">
<blockquote>
<div>
<div> </div>
<br>
</div>
<div class="gmail_extra">
<div class="gmail_quote">
<div>
<div class="gmail-m_4092208918506556890m_7447573976023040723m_-909601598888611618m_-3479522597344256542gmail-h5">2016-10-05 11:45 GMT+02:00 Lorenzo Miniero <span><<a href="mailto:lminiero@gmail.com" onclick="parent.window.location.href='lminiero@gmail.com'; return false;" target="_blank">lminiero@gmail.com</a>></span>:</div>
</div>
<blockquote>
<div>
<div class="gmail-m_4092208918506556890m_7447573976023040723m_-909601598888611618m_-3479522597344256542gmail-h5">
<div>Hi all,
<div> </div>
<div>we've been working on some stress tests of our Janus WebRTC server, which uses libnice for all the ICE related stuff. While it works great in general, we reach a wall much sooner than we expect, and performance start to degrade quite a bit. By doing some digging and diagnostic in the code we wrote and the one we use, we found out something we thought was weird.</div>
<div> </div>
<div>Specifically, we noticed that the internal method that is used to send packets, nice_agent_send_messages_nonblocking (which nice_agent_send itself uses internally) has a lot of code wrapped in an agent_lock()/agent_unlock(), which locks/unlocks a global mutex, and this includes the process of physiclly sending the packet. This means that the whole library is locked until that code is called, which seems overkill. I guess the same happens in other parts of the code as well. I'd understand locking the specific agent if the purpose is thread-safety, but why lock ALL agents at the same time for each packet you send? I tried searching the archives for some info on that, but couldn't find any discussion related to this choice.</div>
<div> </div>
<div>Since I'm already handling thread-safety for most stuff in my own code, is there any way for preventing this from happening? I guess that one possible way could be to retrieve the file descriptor from the agent object and send the packet manually myself, but while this may work in simple (host/prflx/srflx) connectivity, this would definitely not be the case when, for instance, TURN is involved, as that's one of the things the library would take care of for me.</div>
<div> </div>
<div>Looking forward to your thoughts, thanks!<br>
<span class="gmail-m_4092208918506556890m_7447573976023040723m_-909601598888611618m_-3479522597344256542gmail-m_-1552787990672576449HOEnZb"><font color="#888888">Lorenzo</font></span></div>
</div>
</div>
</div>
<span class="gmail-m_4092208918506556890m_7447573976023040723m_-909601598888611618m_-3479522597344256542gmail-">_______________________________________________<br>
nice mailing list<br>
<a href="mailto:nice@lists.freedesktop.org" onclick="parent.window.location.href='nice@lists.freedesktop.org'; return false;" target="_blank">nice@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/nice" target="_blank">https://lists.freedesktop.org/mailman/listinfo/nice</a></span><br>
<br>
</blockquote>
</div>
<br>
<br clear="all">
<br>
<span class="gmail-m_4092208918506556890m_7447573976023040723m_-909601598888611618m_-3479522597344256542gmail-">-- </span>
<div class="gmail-m_4092208918506556890m_7447573976023040723m_-909601598888611618m_-3479522597344256542gmail-m_-1552787990672576449gmail_signature">
<div><span class="gmail-m_4092208918506556890m_7447573976023040723m_-909601598888611618m_-3479522597344256542gmail-">Miguel París Díaz<br>
------------------------------------------------------------------------<br>
Computer/Software engineer.<br>
Researcher and architect in <a href="http://www.kurento.org" target="_blank">http://www.kurento.org</a><br>
<a href="http://twitter.com/mparisdiaz" target="_blank">http://twitter.com/mparisdiaz</a><br>
------------------------------------------------------------------------</span></div>
</div>
<span class="gmail-m_4092208918506556890m_7447573976023040723m_-909601598888611618m_-3479522597344256542gmail-"> </span></div>
</blockquote>
</div>
</div>
</div>
<div>
<div class="gmail-m_4092208918506556890m_7447573976023040723m_-909601598888611618h5"><br>
<br clear="all">
<pre>_______________________________________________
nice mailing list
<a href="mailto:nice@lists.freedesktop.org" onclick="parent.window.location.href='nice@lists.freedesktop.org'; return false;" target="_blank">nice@lists.freedesktop.org</a>
<a href="https://lists.freedesktop.org/mailman/listinfo/nice" target="_blank">https://lists.freedesktop.org/mailman/listinfo/nice</a>
</pre>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</blockquote>
</div>
</div>
<div>
<pre> </pre>
<pre><span class="gmail-HOEnZb"><font color="#888888"><span>-- </span></font></span></pre>
<span class="gmail-HOEnZb"><font color="#888888"><span>Olivier Crête <a href="mailto:olivier.crete@collabora.com" onclick="parent.window.location.href='olivier.crete@collabora.com'; return false;" target="_blank">olivier.crete@collabora.com</a> </span></font></span></div>
</div>
<br></blockquote>
</div>
</div>
</div>
_______________________________________________ nice mailing list nice@lists.freedesktop.org <a href="https://lists.freedesktop.org/mailman/listinfo/nice" target="_blank">https://lists.freedesktop.org/mailman/listinfo/nice</a></div>
</div>
</div>
</div></div>
</blockquote><div><span><pre><pre>-- <br></pre>Olivier Crête
olivier.crete@collabora.com
</pre></span></div></body></html>