[libnice] Sending packets uses a static, global lock?

[Lorenzo Miniero]

Thanks for the context! (and sorry for the delay in this response...)

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! ;-) ).

Thanks,
Lorenzo


2016-10-05 13:09 GMT+02:00 Miguel París Díaz <mparisdiaz at gmail.com>:

> Sorry for the copy-paste error.
>
>
>
> 2016-10-05 13:06 GMT+02:00 Miguel París Díaz <mparisdiaz at gmail.com>:
>
>> Good point Lorenzo!!!
>>
>> This change was added in this commit:
>>
>> commit 2faa76a4840100bebd9532bd47a2755874c5b3a0
>> Author: Youness Alaoui <youness.alaoui at collabora.co.uk>
>> Date:   Tue Jun 16 20:40:33 2009 -0400
>>
>>     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
>>
>> And then the RecMutex was changed by a Mutex in:
>>
>
> commit 1deee69325284c726c3a8380a7d5839a51e20c48
> Author: Olivier Crête <olivier.crete at collabora.com>
> Date:   Mon Feb 24 18:51:31 2014 -0500
>
>     agent: Replace recursive mutex with non-recursive
>
>     This should prevent us from re-adding re-entrancy in the future
>
>
>>
>>
>> 2016-10-05 11:45 GMT+02:00 Lorenzo Miniero <lminiero at gmail.com>:
>>
>>> Hi all,
>>>
>>> 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.
>>>
>>> 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.
>>>
>>> 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.
>>>
>>> Looking forward to your thoughts, thanks!
>>> Lorenzo
>>>
>>> _______________________________________________
>>> nice mailing list
>>> nice at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/nice
>>>
>>>
>>
>>
>> --
>> Miguel París Díaz
>> ------------------------------------------------------------------------
>> Computer/Software engineer.
>> Researcher and architect in http://www.kurento.org
>> http://twitter.com/mparisdiaz
>> ------------------------------------------------------------------------
>>
>
>
>
> --
> Miguel París Díaz
> ------------------------------------------------------------------------
> Computer/Software engineer.
> Researcher and architect in http://www.kurento.org
> http://twitter.com/mparisdiaz
> ------------------------------------------------------------------------
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/nice/attachments/20161012/84c9601a/attachment.html>