New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 5139 link

Starred by 4 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

WebRTC should use async DNS resolution function

Reported by guoweis@chromium.org, Oct 29 2015

Issue description

We have observed that a hang on android where the thread is waiting the getaddrinfo thread. 

Whether to wait or not is determined by AsyncResolver::Destroy(fwait). Currently, only stunport specifies true in this case and it's where the hang is.

However, in theory, we should use true to make sure the dll doesn't get unloaded before the thread dies (which will cause access violation).

On android, there is currently no async dns function though.
 
Project Member

Comment 1 by guoweis@webrtc.org, Oct 29 2015

Labels: Area-Network
Project Member

Comment 2 by juberti@webrtc.org, Oct 30 2015

I believe Chrome has its own async DNS, so this would primarily be for 
- Android (which I don't think has async DNS, at least not at NDK level), and iOS.
- iOS, which has CFHostStartInfoResolution

However I don't think that either Android or iOS support shared libraries, so this may not be that critical to address.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 30 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/8a4f547dadecb1ec4e1647e0ca1ad74be3048801

commit 8a4f547dadecb1ec4e1647e0ca1ad74be3048801
Author: Guo-wei Shieh <guoweis@webrtc.org>
Date: Fri Oct 30 16:12:34 2015

Hang on android when DNS resolution is not done

BUG=webrtc:5139
R=juberti@google.com
TBR=juberti@webrtc.org

Review URL: https://codereview.webrtc.org/1429493009 .

Cr-Commit-Position: refs/heads/master@{#10463}

[modify] http://crrev.com/8a4f547dadecb1ec4e1647e0ca1ad74be3048801/webrtc/p2p/base/stunport.cc

Project Member

Comment 4 by pthatcher@webrtc.org, Nov 9 2015

Labels: EngTriaged IceBox
Project Member

Comment 5 by tnakamura@webrtc.org, Nov 17 2015

guoweis@ - do you consider this fixed with https://codereview.webrtc.org/1429493009?
no, it's just mitigating the issue.
Project Member

Comment 7 by pthatcher@webrtc.org, Nov 8 2016

Labels: Pri-3
Project Member

Comment 8 by anatolid@chromium.org, Dec 5 2016

Status: Available (was: Unconfirmed)
Project Member

Comment 9 by nisse@webrtc.org, Jun 19 2017

This code also leaks memory, resolvers are allocated by 

  socket_factory_->CreateAsyncResolver(), 

then raw pointers are inserted into a mapping, and never deallocated. This made the asan bot fail on my cl https://codereview.webrtc.org/2915253002/ (unclear why it didn't fail earlier, maybe there's some suppression which I haven't found and which no longer matches after my change)?

The obvious way to fix would be to use unique_ptrs in the mapping. However, that will have a similar effect as changing the wait argument back to true, since destroying the async resolver implies waiting for termination of its worker thread.
Project Member

Comment 10 by nisse@webrtc.org, Jun 19 2017

Hmm, it seems my comment  #9 was based on a misunderstanding. It seems the intention is that Destroy(false) should imply delayed deletion of the object, as soon as the worker thread is finished.
Project Member

Comment 11 by sheriffbot@chromium.org, Sep 6

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.


Project Member

Comment 12 by anatolid@webrtc.org, Nov 1

Status: Available (was: Untriaged)
[bulk-edit] Setting status to Available since it's likely that this issue shouldn't be archived yet. Also changing Pri to 3 due to long period of inactivity (indicating low priority).

Comment 13 Deleted

Project Member

Comment 14 by zstein@webrtc.org, Nov 27

I don't think there is actually a leak. The semantics are a bit confusing here though. There are two implementations of AsyncResolverInterface:

The WebRTC version (in nethelpers.h) inherits from SignalThread and deletes itself once it has finished performing its work.
The Chromium version (in ipc_socket_factory.cc) explicitly calls delete in a method triggered by SignalDone.

Please let me know if you observe something different happening.

Sign in to add a comment