Clarification: purpose of conn_lock & timeout configuration

[ofacklam]

Hello @paullouisageneau!

Thanks a lot for the great libdatachannel and libjuice libraries.

I recently came across an interesting behavior in our application which was related to the conn_lock handling of the libjuice agent.
Basically, the flow was as follows:

1. The application receives the remote offer and some ice candidates.
2. libdatachannel PeerConnection starts the ICE agent with a STUN server.
3. At this point the internet connection breaks away.
4. Even though here agent_resolve_servers is started in a separate thread, it still locks the connection with conn_lock. However, the STUN server address cannot be resolved, so this is stuck for ~30 seconds.
5. In the meantime, the app tries to addRemoteCandidate, but this need the connection lock.

So now the app and the entire system is blocked waiting for the address lookup to time out. Nothing critical, but feels a bit weird.

My question would be:

• is this expected behavior?
• is it really required to have the conn_lock for the whole duration of resolving the servers?
• or is there a way to configure the address lookup timeout to make this process snappier?

Thanks
Oliver

[rcvangemert]

Also, Thanks a lot for the great libjuice library. your questions prompts me to ask why it takes 25sec for juice to report connection disconnect. Also sometimes my app crashes when I have back to back connection requests even though I call juice_destroy before restarting the connection process. If I wait before reconnecting it is a lot more stable. I may be the cause of this problem! Interesting the crash dump ALWAYS has this sequence with the last entry referring to (conn_lock+24). 2024-08-14 20:07:28.568 2757-2757 DEBUG crash_dump64 A pid: 29324, tid: 29324, name: ert.videoAssist >>> com.vangemert.videoAssist <<< 2024-08-14 20:07:28.568 2757-2757 DEBUG crash_dump64 A #00 pc 000000000002a900 /data/app/~~TJWILplj8EymwfFmWbIgRA==/com.vangemert.videoAssist-GYMlKgPVxYytuqQgv88zHw==/base.apk!libjuice.so (offset 0xc74000) (conn_lock+24) (BuildId: b52aefb3a10734dfb3d36877c6b63ff9dedb8677) 2024-08-14 20:07:28.568 2757-2757 DEBUG crash_dump64 A #1 pc 0000000000022544 /data/app/~~TJWILplj8EymwfFmWbIgRA==/com.vangemert.videoAssist-GYMlKgPVxYytuqQgv88zHw==/base.apk!libjuice.so (offset 0xc74000) (agent_set_remote_description+44) (BuildId: b52aefb3a10734dfb3d36877c6b63ff9dedb8677) 2024-08-14 20:07:28.568 2757-2757 DEBUG crash_dump64 A #2 pc 0000000000035210 /data/app/~~TJWILplj8EymwfFmWbIgRA==/com.vangemert.videoAssist-GYMlKgPVxYytuqQgv88zHw==/base.apk!libjuice.so (offset 0xc74000) (juice_set_remote_description+80) (BuildId: b52aefb3a10734dfb3d36877c6b63ff9dedb8677) 2024-08-14 20:07:28.568 2757-2757 DEBUG crash_dump64 A #3 pc 000000000001e2ec /data/app/~~TJWILplj8EymwfFmWbIgRA==/com.vangemert.videoAssist-GYMlKgPVxYytuqQgv88zHw==/base.apk!libjuice.so (offset 0xc74000) (nativeSetRemoteDescription+140) (BuildId: b52aefb3a10734dfb3d36877c6b63ff9dedb8677) 2024-08-14 20:07:28.568 2757-2757 DEBUG crash_dump64 A #9 pc 0000000000032610 /data/app/~~TJWILplj8EymwfFmWbIgRA==/com.vangemert.videoAssist-GYMlKgPVxYytuqQgv88zHw==/base.apk (com.vangemert.videoAssist.Juice.access$600+0) 2024-08-14 20:07:28.569 2757-2757 DEBUG crash_dump64 A #14 pc 00000000000322fc /data/app/~~TJWILplj8EymwfFmWbIgRA==/com.vangemert.videoAssist-GYMlKgPVxYytuqQgv88zHw==/base.apk (com.vangemert.videoAssist.Juice$2.onDataChange+0) 2024-08-14 20:07:28.569 2757-2757 DEBUG crash_dump64 A #19 pc 00000000003f6b6c /data/app/~~TJWILplj8EymwfFmWbIgRA==/com.vangemert.videoAssist-GYMlKgPVxYytuqQgv88zHw==/base.apk (com.google.firebase.database.core.ValueEventRegistration.fireEvent+0) 2024-08-14 20:07:28.569 2757-2757 DEBUG crash_dump64 A #24 pc 00000000003ff250 /data/app/~~TJWILplj8EymwfFmWbIgRA==/com.vangemert.videoAssist-GYMlKgPVxYytuqQgv88zHw==/base.apk (com.google.firebase.database.core.view.DataEvent.fire+0) 2024-08-14 20:07:28.569 2757-2757 DEBUG crash_dump64 A #29 pc 00000000003ff704 /data/app/~~TJWILplj8EymwfFmWbIgRA==/com.vangemert.videoAssist-GYMlKgPVxYytuqQgv88zHw==/base.apk (com.google.firebase.database.core.view.EventRaiser$1.run+0) Thanks, Robert

[paullouisageneau]

your questions prompts me to ask why it takes 25sec for juice to report connection disconnect.

This is expected and unrelated. ICE offers UDP connectivity and does not introduce any explicit connection close mechanism, disconnection is only reported after a timeout if the remote agent stops answering keepalives. If you want to close connections explicitly, you need to implement it in your transport protocol. That's what WebRTC data channels do for instance, see https://github.com/paullouisageneau/libdatachannel.

Also sometimes my app crashes when I have back to back connection requests even though I call juice_destroy before restarting the connection process. If I wait before reconnecting it is a lot more stable. I may be the cause of this problem!

Interesting the crash dump ALWAYS has this sequence with the last entry referring to (conn_lock+24).

I think the crash happens in conn_lock() only because it is the first function called which access the agent struct. It is probable the agent pointer passed to juice_set_remote_description() is invalid.

[paullouisageneau]

My question would be:

• is this expected behavior?
• is it really required to have the conn_lock for the whole duration of resolving the servers?

No, this is an oversight. conn_lock is used to lock the connection thread for this agent, depending on the concurrency mode, and the lock should be released while resolving the addresses as it can take a while.

• or is there a way to configure the address lookup timeout to make this process snappier?

Sadly the getaddrinfo() method does not allow the caller to specify a timeout, it is a system setting (for instance it can be set it /etc/resolve.conf on Linux).

[paullouisageneau]

@ofacklam I fixed locking in #267, thank you for reporting.

[ofacklam]

Amazing, thanks for the quick response and fixing!

[rcvangemert]

Understand now about keepalives. I'll check my code again regarding the intermittent crash. Thanks for your input.

…

On Thu, 15 Aug 2024, 19:33 Paul-Louis Ageneau, ***@***.***> wrote: your questions prompts me to ask why it takes 25sec for juice to report connection disconnect. This is expected and unrelated. ICE offers UDP connectivity and does not introduce any explicit connection close mechanism, disconnection is only reported after a timeout if the remote agent stops answering keepalives. If you want to close connections explicitly, you need to implement it in your transport protocol. That's what WebRTC data channels do for instance, see https://github.com/paullouisageneau/libdatachannel. Also sometimes my app crashes when I have back to back connection requests even though I call juice_destroy before restarting the connection process. If I wait before reconnecting it is a lot more stable. I may be the cause of this problem! Interesting the crash dump ALWAYS has this sequence with the last entry referring to (conn_lock+24). I think the crash happens in conn_lock() only because it is the first function called which access the agent struct. It is probable the agent pointer passed to juice_set_remote_description() is invalid. — Reply to this email directly, view it on GitHub <#264 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABGSW3AHYR7VF2QLK4GRURDZRRYW3AVCNFSM6AAAAABMOOU6XKVHI2DSMVQWIX3LMV43URDJONRXK43TNFXW4Q3PNVWWK3TUHMYTAMZUGU3DOMI> . You are receiving this because you commented.Message ID: ***@***.*** com>