diff options
| author | eroman@chromium.org <eroman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-05-05 23:25:29 +0000 |
|---|---|---|
| committer | eroman@chromium.org <eroman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-05-05 23:25:29 +0000 |
| commit | 6627ef6ed1e99731c6c2f2e3a87179fc6448790e (patch) | |
| tree | 7bec24d1a585fb4666f79b9fcbb87bd7333cc202 /chrome/browser/net | |
| parent | ea14138640443f60808ebb388d7dbdba630252d6 (diff) | |
| download | chromium_src-6627ef6ed1e99731c6c2f2e3a87179fc6448790e.zip chromium_src-6627ef6ed1e99731c6c2f2e3a87179fc6448790e.tar.gz chromium_src-6627ef6ed1e99731c6c2f2e3a87179fc6448790e.tar.bz2 | |
Fix a unit-test that was crashing on valgrind bot.
The problem is that ClientSocketPoolBaseHelper holds a pointer to a task, and tries to call a method on that pointer during destruction.
However, if destruction of ClientSocketPoolBaseHelper occurs during message loop destruction, then pending tasks are deleted, so this "backup_task" pointer that it accesses may be invalid.
I worked around the problem here by avoiding this destruction code-path in the unit-test. It would be good to make ClientSocketPoolBaseHelper able to handle this case though.
BUG=43291
Review URL: http://codereview.chromium.org/1920003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@46521 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/net')
| -rw-r--r-- | chrome/browser/net/connection_tester_unittest.cc | 19 |
1 files changed, 15 insertions, 4 deletions
diff --git a/chrome/browser/net/connection_tester_unittest.cc b/chrome/browser/net/connection_tester_unittest.cc index b9cf2ca..4b356b1 100644 --- a/chrome/browser/net/connection_tester_unittest.cc +++ b/chrome/browser/net/connection_tester_unittest.cc @@ -116,12 +116,12 @@ TEST_F(ConnectionTesterTest, DeleteWhileInProgress) { scoped_refptr<HTTPTestServer> server = HTTPTestServer::CreateForkingServer(L"net/data/url_request_unittest/"); - ConnectionTester tester(&test_delegate_); + scoped_ptr<ConnectionTester> tester(new ConnectionTester(&test_delegate_)); // Start the test suite on URL "echoall". // TODO(eroman): Is this URL right? GURL url = server->TestServerPage("echoall"); - tester.RunAllTests(url); + tester->RunAllTests(url); MessageLoop::current()->RunAllPending(); @@ -130,8 +130,19 @@ TEST_F(ConnectionTesterTest, DeleteWhileInProgress) { EXPECT_EQ(0, test_delegate_.completed_connection_test_experiment_count()); EXPECT_EQ(0, test_delegate_.completed_connection_test_suite_count()); - // Note here that we don't wait for the tests to complete - // (so |tester| will be deleted upon leaving this scope. + // Delete the ConnectionTester while it is in progress. + tester.reset(); + + // Drain the tasks on the message loop. + // + // Note that we cannot simply stop the message loop, since that will delete + // any pending tasks instead of running them. This causes a problem with + // net::ClientSocketPoolBaseHelper, since the "Group" holds a pointer + // |backup_task| that it will try to deref during the destructor, but + // depending on the order that pending tasks were deleted in, it might + // already be invalid! See http://crbug.com/43291. + MessageLoop::current()->PostTask(FROM_HERE, new MessageLoop::QuitTask()); + MessageLoop::current()->Run(); } } // namespace |