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

Issue 4149 link

Starred by 7 users

Issue metadata

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



Sign in to add a comment

rtc_unittests AsyncHttpRequestTest.TestCancel leaks memory under ASan

Project Member Reported by kjellander@webrtc.org, Jan 7 2015

Issue description

What steps will reproduce the problem?
1. Checkout WebRTC on Linux
2. GYP_DEFINES='asan=1 lsan=1 release_extra_cflags=-g use_allocator=none' webrtc/build/gyp_webrtc
3. ninja -C out/Release rtc_unittests
4. ASAN_OPTIONS=detect_leaks=1 out/Debug/rtc_unittests --gtest_filter=AsyncHttpRequestTest.TestCancel

What is the expected result?
The tests should pass.

What do you see instead?
A leak is reported:

See http://build.chromium.org/p/client.webrtc.fyi/builders/Linux%20Asan%20%28parallel%29/builds/19/steps/rtc_unittests/logs/stdio for an example:

=================================================================
==14640==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x4e9509 in operator new(unsigned long) (/mnt/data/b/build/slave/linux_asan/build/src/out/Release/rtc_unittests+0x4e9509)
    #1 0xc5cb68 in rtc::Thread::Start(rtc::Runnable*) webrtc/base/thread.cc:228:3
    #2 0xc226dd in rtc::SignalThread::Start() webrtc/base/signalthread.cc:55:5
    #3 0xbbb323 in StartDNSLookup webrtc/base/httpclient.cc:323:3
    #4 0xbbb323 in rtc::HttpClient::connect() webrtc/base/httpclient.cc:401
    #5 0xbbc03b in rtc::HttpClient::start() webrtc/base/httpclient.cc:395:3
    #6 0xb940a0 in rtc::AsyncHttpRequest::LaunchRequest() webrtc/base/asynchttprequest.cc:113:3
    #7 0xc226d2 in rtc::SignalThread::Start() webrtc/base/signalthread.cc:54:5
    #8 0x4f09b1 in rtc::AsyncHttpRequestTest_TestCancel_Test::TestBody() webrtc/base/asynchttprequest_unittest.cc:211:3
    #9 0xdf5829 in HandleExceptionsInMethodIfSupported<testing::Test, void> chromium/src/testing/gtest/src/gtest.cc:2418:12
    #10 0xdf5829 in testing::Test::Run() chromium/src/testing/gtest/src/gtest.cc:2434
    #11 0xdf6e2b in testing::TestInfo::Run() chromium/src/testing/gtest/src/gtest.cc:2610:5
    #12 0xdf7f65 in testing::TestCase::Run() chromium/src/testing/gtest/src/gtest.cc:2728:5
    #13 0xe0b57e in testing::internal::UnitTestImpl::RunAllTests() chromium/src/testing/gtest/src/gtest.cc:4591:11
    #14 0xe0ab71 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> chromium/src/testing/gtest/src/gtest.cc:2418:12
    #15 0xe0ab71 in testing::UnitTest::Run() chromium/src/testing/gtest/src/gtest.cc:4209
    #16 0xc979b9 in RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2326:10
    #17 0xc979b9 in main webrtc/base/unittest_main.cc:92
    #18 0x7f74bb0e076c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226

-----------------------------------------------------
Suppressions used:
  count      bytes template
      1         72 libnss3
-----------------------------------------------------

SUMMARY: AddressSanitizer: 16 byte(s) leaked in 1 allocation(s).


The leak only seems to happen when running the test as a single test (which is what happens when we run the tests using the gtest-parallel script).

Please use labels and text to provide additional information.

 
Project Member

Comment 1 by kjellander@webrtc.org, Jan 7 2015

Labels: OS-
Correction: I'm unable to repro locally on my Linux machine and the command line in the bug is missing the suppression configuration.

The full command line that is similar to the bot is:
ASAN_OPTIONS="symbolize=1 external_symbolizer_path=`pwd`/third_party/llvm-build/Release+Asserts/bin/llvm-symbolizer strip_path_prefix=out/" LSAN_OPTIONS="symbolize=1 external_symbolizer_path=`pwd`/third_party/llvm-build/Release+Asserts/bin/llvm-symbolizer strip_path_prefix=out/Release/../../ suppressions=`pwd`/lsan/suppressions.txt print_suppressions=1" out/Release/rtc_unittests --gtest_filter=AsyncHttpRequestTest.TestCancel

But it passes for me.
Project Member

Comment 2 by kjellander@webrtc.org, Jan 7 2015

Labels: -OS-
Project Member

Comment 3 by pbos@webrtc.org, Jan 7 2015

Cc: juberti@webrtc.org
So this looks timing dependent, but also very weird. I can't repro it regularly, or in parallel with 100 runs, but if I add so that I have 200 workers (lots of contention) it seems to repro although rarely. Looks more common on bots:

------- I'm printing things with the following patch (ThreadInit* init = new ThreadInit; is the object that leaks):

pbos@deimos:~/webrtc/src (master)$ git diff
diff --git a/webrtc/base/thread.cc b/webrtc/base/thread.cc
index aa2442f..bf68c07 100644
--- a/webrtc/base/thread.cc
+++ b/webrtc/base/thread.cc
@@ -226,6 +226,7 @@ bool Thread::Start(Runnable* runnable) {
   ThreadManager::Instance();
 
   ThreadInit* init = new ThreadInit;
+  printf("thread (%p): spawning init %p\n", this, init);
   init->thread = this;
   init->runnable = runnable;
 #if defined(WEBRTC_WIN)
@@ -279,11 +280,14 @@ bool Thread::Start(Runnable* runnable) {
   }
 #endif  // !defined(__native_client__)
 
+  printf("thread (%p): %p:%d calling pthread_create\n", this, init, __LINE__);
   int error_code = pthread_create(&thread_, &attr, PreRun, init);
   if (0 != error_code) {
+  printf("thread (%p) %p:%d pthread_create failed\n", this, init, __LINE__);
     LOG(LS_ERROR) << "Unable to create pthread, error " << error_code;
     return false;
   }
+  printf("thread (%p): %p:%d pthread_create done\n", this, init, __LINE__);
   running_.Set();
 #endif
   return true;
@@ -312,6 +316,7 @@ void Thread::SafeWrapCurrent() {
 }
 
 void Thread::Join() {
+  printf("thread (%p): Joining!\n", this);
   if (running()) {
     ASSERT(!IsCurrent());
     if (Current() && !Current()->blocking_calls_allowed_) {
@@ -376,6 +381,7 @@ void SetThreadName(DWORD dwThreadID, LPCSTR szThreadName) {
 #endif  // WEBRTC_WIN
 
 void* Thread::PreRun(void* pv) {
+  printf("%p: PreRun has started!\n", pv);
   ThreadInit* init = static_cast<ThreadInit*>(pv);
   ThreadManager::Instance()->SetCurrentThread(init->thread);
 #if defined(WEBRTC_WIN)


------- Which gives me the following:

Note: Google Test filter = AsyncHttpRequestTest.TestCancel
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from AsyncHttpRequestTest
[ RUN      ] AsyncHttpRequestTest.TestCancel
[000:000] HttpRequest start: localhost/get
[000:001] HttpRequest cancelled
thread (0x61700000f5e0): spawning init 0x60200001a3d0
thread (0x61700000f5e0): 0x60200001a3d0:283 calling pthread_create
thread (0x61700000f5e0): 0x60200001a3d0:290 pthread_create done
thread (0x62d00000a460): spawning init 0x60200001a3b0
thread (0x62d00000a460): 0x60200001a3b0:283 calling pthread_create
thread (0x62d00000a460): 0x60200001a3b0:290 pthread_create done
thread (0x62d00000a460): Joining!
0x60200001a3b0: PreRun has started!
thread (0x62d00000a460): Joining!
thread (0x62d00000a460): Joining!
[       OK ] AsyncHttpRequestTest.TestCancel (7 ms)
[----------] 1 test from AsyncHttpRequestTest (7 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (9 ms total)
[  PASSED  ] 1 test.


------- Which gives a couple of very weird things and corresponding questions:

One: Why are both 'init' objects allocated at the same address? I can't see this being deleted anywhere? (WTF). It might have been printed out of order, so 0x62d00000a460 actually finished its PreRun before, handing back the memory.

Two: Why isn't thread 0x61700000f5e0 joining? Can a thread be deleted without being joined?

Three: Why is PreRun only called for one of these objects before we finish? (PreRun deletes init, not being run explains the leak.)

To me it looks like thread 0x61700000f5e0 hasn't run its PreRun (posix thread hasn't started) before the thread object is deleted. Is this leaky by design somehow?

-------

PTAL this is blocking running ASAN in parallel. It's also quite tricky to unnest. I'm running this to repro the bug. Running just now I get 11/1000 failures:

(export ASAN_OPTIONS=detect_leaks=1; export LSAN_OPTIONS="symbolize=1 external_symbolizer_path=`pwd`/third_party/llvm-build/Release+Asserts/bin/llvm-symbolizer strip_path_prefix=out/Release/../../ suppressions=tools/lsan/suppressions.txt print_suppressions=1"; ninja-webrtc-asan && third_party/gtest-parallel/gtest-parallel -r 1000 -w200 out/Debug/rtc_unittests --gtest_filter=*AsyncHttp*TestCancel)
Project Member

Comment 4 by pbos@webrtc.org, Jan 7 2015

ninja-webrtc-asan is what I use to build asan, a normal build is done using:

GYP_DEFINES=asan=1 gclient runhooks, and building with ninja as usual.
Project Member

Comment 5 by juberti@webrtc.org, Jan 7 2015

Labels: Area-Network
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 8 2015

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

commit 5f09564354871bfbdb5ce24f7a95c6348a8e2c7b
Author: kjellander@webrtc.org <kjellander@webrtc.org>
Date: Thu Jan 08 10:45:59 2015

Suppress AsyncHttpRequestTest.TestCancel leak for LSan

This test fails when run with gtest-parallel on the
ASan bot (that also runs LSan):
http://build.chromium.org/p/client.webrtc.fyi/builders/Linux%20Asan%20%28parallel%29/builds/27

I'm unable to reproduce this locally but it's
obviously failing on the bots.

BUG=4149
R=pbos@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/41419004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@8021 4adac7df-926f-26a2-2b94-8c16560cd09d

[modify] http://crrev.com/5f09564354871bfbdb5ce24f7a95c6348a8e2c7b/tools/lsan/suppressions.txt

Project Member

Comment 7 by pthatcher@webrtc.org, Jan 12 2015

Labels: EngTriaged Mstone-44
Owner: guoweis@webrtc.org
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 4 2015

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

commit d415629de78b84bf539fda7f634996e159b0dcaa
Author: Peter Thatcher <pthatcher@chromium.org>
Date: Fri Sep 04 11:21:07 2015

Remove AsyncHttpRequest, AutoPortAllocator, ConnectivityChecker, and HttpPortAllocator.

BUG=webrtc:4149, webrtc:4456
R=deadbeef@webrtc.org, pbos@webrtc.org

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

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

[modify] http://crrev.com/d415629de78b84bf539fda7f634996e159b0dcaa/webrtc/base/BUILD.gn
[modify] http://crrev.com/d415629de78b84bf539fda7f634996e159b0dcaa/webrtc/base/asynchttprequest.cc
[modify] http://crrev.com/d415629de78b84bf539fda7f634996e159b0dcaa/webrtc/base/asynchttprequest.h
[delete] http://crrev.com/2f9fd5ddb9097054b71fce20ba24bc34b2f084c0/webrtc/base/asynchttprequest_unittest.cc
[modify] http://crrev.com/d415629de78b84bf539fda7f634996e159b0dcaa/webrtc/base/base.gyp
[modify] http://crrev.com/d415629de78b84bf539fda7f634996e159b0dcaa/webrtc/base/base_tests.gyp
[delete] http://crrev.com/2f9fd5ddb9097054b71fce20ba24bc34b2f084c0/webrtc/p2p/client/connectivitychecker.cc
[delete] http://crrev.com/2f9fd5ddb9097054b71fce20ba24bc34b2f084c0/webrtc/p2p/client/connectivitychecker.h
[delete] http://crrev.com/2f9fd5ddb9097054b71fce20ba24bc34b2f084c0/webrtc/p2p/client/connectivitychecker_unittest.cc
[modify] http://crrev.com/d415629de78b84bf539fda7f634996e159b0dcaa/webrtc/p2p/client/httpportallocator.cc
[modify] http://crrev.com/d415629de78b84bf539fda7f634996e159b0dcaa/webrtc/p2p/client/httpportallocator.h
[modify] http://crrev.com/d415629de78b84bf539fda7f634996e159b0dcaa/webrtc/p2p/client/portallocator_unittest.cc
[modify] http://crrev.com/d415629de78b84bf539fda7f634996e159b0dcaa/webrtc/p2p/p2p.gyp
[modify] http://crrev.com/d415629de78b84bf539fda7f634996e159b0dcaa/webrtc/p2p/p2p_tests.gypi

Project Member

Comment 9 by pthatcher@webrtc.org, Feb 8 2016

Labels: -Mstone-44
Owner: deadbeef@webrtc.org
I think we can just delete AsyncHttpRequest and HttpPortAllocator.
Project Member

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

Labels: Pri-3
Project Member

Comment 11 by deadbeef@chromium.org, Apr 3 2018

Owner: ----
Status: Available (was: Assigned)
Clearing owner and setting status to Available, since there haven't been any updates for > 1 year. Will be assigned again once priority is high enough.

Sign in to add a comment