[Nice] Patch for TURN CreatePermission

Thu Mar 21 04:13:33 PDT 2013

Hi,
regarding this old patch, there is still a problem.  The list  pending_permission contains all the pending TURN CreatePermission. When a new permission is stored in the list, a new timeout reminder is created and stored in the variable tick_source_create_permission overwriting the old value. 
I fixed this problem by creating just one timeout timer, the one that expires first. When that  timer expires, a new one is created with the next timeout value.

What do you think?

I send the patch for review. 

Regards,
Livio

-----Original Message-----
From: nice-bounces+livio.madaro=telecomitalia.it at lists.freedesktop.org [mailto:nice-bounces+livio.madaro=telecomitalia.it at lists.freedesktop.org] On Behalf Of Madaro Livio
Sent: Tuesday, November 20, 2012 10:39 AM
To: nice at lists.freedesktop.org
Subject: Re: [Nice] Patch for TURN CreatePermission

Thanks for reviewing and fixing the patch. I had a look at the final result and I think it's OK.

Regards,
Livio

-----Original Message-----
From: nice-bounces+livio.madaro=telecomitalia.it at lists.freedesktop.org [mailto:nice-bounces+livio.madaro=telecomitalia.it at lists.freedesktop.org] On Behalf Of Youness Alaoui
Sent: martedì 20 novembre 2012 0.22
To: nice at lists.freedesktop.org
Subject: Re: [Nice] Patch for TURN CreatePermission

Thanks,

I had a couple of issues with the patch, but I fixed them and committed, have a look at the final result :
http://cgit.collabora.com/git/libnice.git/commit/?id=d977c25410d89ab9028228c247b7751a965a0fb4

Changes :
- Removed trailing whitespaces and other small coding style inconsistencies
- Used the GList iterator in a deletion-safe way without using get_nth
- renamed variable to be less than 100 characters long :p

On 11/09/2012 08:37 AM, Madaro Livio wrote:
> OK. I think you are right. I remove current_create_permission_msg and I fix the memory leak issue.
> 
> I send the new patch for review.
> 
> Livio
> 
> 
> -----Original Message-----
> From: nice-bounces+livio.madaro=telecomitalia.it at lists.freedesktop.org 
> [mailto:nice-bounces+livio.madaro=telecomitalia.it at lists.freedesktop.o
> rg] On Behalf Of Youness Alaoui
> Sent: giovedì 8 novembre 2012 19.46
> To: nice at lists.freedesktop.org
> Subject: Re: [Nice] Patch for TURN CreatePermission
> 
> Hi Madaro,
> 
> Sorry for not replying earlier, I was travelling.
> Thanks for the patch. That's a good point you got there, however, the 
> patch kept the priv->current_create_permission_msg, but I think it 
> should be removed. There is no reason to keep the message twice (in 
> the list and in the current_*) and it gets confusing, you do a 
> delete_link and g_free the current_create_permission_msg in the code, 
> but in the socket_close, you removed the g_free.. also, you call 
> g_list_free but you should call g_list_free_full in order to also free 
> the messages contained within the list, otherwise you get a memleak.
> I don't have time right now to fix all of that myself, could you redo 
> the patch and send me a new one for review?
> 
> Thank you,
> Youness.
> 
> On 11/02/2012 08:59 AM, Madaro Livio wrote:
>> Hi, I found a problem using TURN.
>>
>> Libnice sends a TURN message CreatePermission Request for each remote 
>> candidate added by nice_agent_set_remote_candidates. The TURN server 
>> replies with a CreatePermission Response and libnice matches the 
>> Request and the Response using the message id. The problem is that 
>> libnice uses a single variable to store the sent messages 
>> (current_create_permission_msg).If a new CreatePermission Request is 
>> sent before receiving the Response of the previous one, the variable 
>> current_create_permission_msgis overwritten and the Response of the first message will never match the Request.
>>
>>
>>
>> I send a patch for review. It adds a GList variable
>> (pending_create_permission_messages) to store all the 
>> CreatePermission Request waiting for a Response.
>>
>>
>>
>> Regards,
>>
>> Livio
>>
>>
>>
>> Questo messaggio e i suoi allegati sono indirizzati esclusivamente 
>> alle persone indicate. La diffusione, copia o qualsiasi altra azione 
>> derivante dalla conoscenza di queste informazioni sono rigorosamente 
>> vietate. Qualora abbiate ricevuto questo documento per errore siete 
>> cortesemente pregati di darne immediata comunicazione al mittente e di provvedere alla sua distruzione, Grazie.
>>
>> /This e-mail and any attachments// is //confidential and may contain 
>> privileged information intended for the addressee(s) only. 
>> Dissemination, copying, printing or use by anybody else is 
>> unauthorised. If you are not the intended recipient, please delete 
>> this message and any attachments and advise the sender by return 
>> e-mail, Thanks./
>>
>> *rispetta l'ambienteRispetta l'ambiente. Non stampare questa mail se 
>> non è
>> necessario.*
>>
>>
>>
>> _______________________________________________
>> nice mailing list
>> nice at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/nice
>>
> 
> 
> 
> Questo messaggio e i suoi allegati sono indirizzati esclusivamente alle persone indicate. La diffusione, copia o qualsiasi altra azione derivante dalla conoscenza di queste informazioni sono rigorosamente vietate. Qualora abbiate ricevuto questo documento per errore siete cortesemente pregati di darne immediata comunicazione al mittente e di provvedere alla sua distruzione, Grazie.
> 
> This e-mail and any attachments is confidential and may contain privileged information intended for the addressee(s) only. Dissemination, copying, printing or use by anybody else is unauthorised. If you are not the intended recipient, please delete this message and any attachments and advise the sender by return e-mail, Thanks.
> 
> 
> 
> _______________________________________________
> nice mailing list
> nice at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nice
> 

_______________________________________________
nice mailing list
nice at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nice
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-timer-for-TURN-CreatePermission.patch
Type: application/octet-stream
Size: 4636 bytes
Desc: 0001-Fix-timer-for-TURN-CreatePermission.patch
URL: <http://lists.freedesktop.org/archives/nice/attachments/20130321/2a23bedc/attachment.obj>