Force non-joinable SimpleThreads to be leaked.

base/threading/simple_thread_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <memory>
 
 #include "base/atomic_sequence_num.h"
+#include "base/debug/leak_annotations.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/test/gtest_util.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/simple_thread.h"
 #include "base/time/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace base {
 
@@ skipping 121 matching lines @@
 
   EXPECT_TRUE(event.IsSignaled());
   thread.Join();
 
   // We keep the name and tid, even after the thread is gone.
   EXPECT_EQ(thread.name_prefix(), "event_waiter");
   EXPECT_EQ(thread.name(),
             std::string("event_waiter/") + IntToString(thread.tid()));
 }
 
-TEST(SimpleThreadTest, JoinNonJoinableThreadDeath) {
+TEST(SimpleThreadTest, StartNonJoinableThreadAndDieOnJoin) {
+  SleepRunner runner;

[sdefresne, 2016/08/05 16:48:33]

I think SleepRunner also needs to be leaked as DelegateSimpleThread::Run will
call SleepRunner::Run which is a virtual method.

[gab, 2016/08/05 17:28:33]

Good point, for DelegateSimpleThread, the Delegate also needs to be leaked this
makes me think that it's okay to delete non-joinable SimpleThreads when they
have a Delegate, so long as the Delegate is deleted.

This is in fact my proposal to eventually not force base::Thread to be leaked
under similar circumstances (but it currently doesn't have a Delegate):
https://bugs.chromium.org/p/chromium/issues/detail?id=629139#c14

I'll change the CL to only force leaking the Delegate.

+
+  SimpleThread::Options options;
+  options.joinable = false;
+  DelegateSimpleThread* thread =
+      new DelegateSimpleThread(&runner, "noop_runner", options);
+  // Non-joinable SimpleThreads have to be leaked.
+  ANNOTATE_LEAKING_OBJECT_PTR(thread);
+
+  EXPECT_FALSE(thread->HasBeenStarted());
+  thread->Start();
+  EXPECT_TRUE(thread->HasBeenStarted());
+  EXPECT_FALSE(thread->HasBeenJoined());
+
+  EXPECT_DCHECK_DEATH(thread->Join());
+}
+
+TEST(SimpleThreadTest, NonJoinableThreadDeathOnDestruction) {
   SleepRunner runner;
   SimpleThread::Options options;
   options.joinable = false;
-  DelegateSimpleThread thread(&runner, "noop_runner", options);
+  DelegateSimpleThread* thread =
+      new DelegateSimpleThread(&runner, "noop_runner", options);
+  ANNOTATE_LEAKING_OBJECT_PTR(thread);
 
-  thread.Start();
-  EXPECT_DCHECK_DEATH(thread.Join());
-}
-
-TEST(SimpleThreadTest, DeleteNonJoinableWithoutJoinIsSafe) {
-  SleepRunner runner;
-  SimpleThread::Options options;
-  options.joinable = false;
-  DelegateSimpleThread thread(&runner, "noop_runner", options);
-  thread.Start();
-  // Nothing blows up when |thread| goes out of scope.
+  thread->Start();
+  EXPECT_DCHECK_DEATH({ delete thread; });
 }
 
 TEST(SimpleThreadTest, ThreadPool) {
   AtomicSequenceNumber seq;
   SeqRunner runner(&seq);
   DelegateSimpleThreadPool pool("seq_runner", 10);
 
   // Add work before we're running.
   pool.AddWork(&runner, 300);
 
@@ skipping 16 matching lines @@
   VerifyPoolRunner verifier(&seq2, 9, &event);
   pool.Start();
 
   pool.AddWork(&verifier, 10);
 
   pool.JoinAll();
   EXPECT_EQ(seq2.GetNext(), 10);
 }
 
 }  // namespace base