Add joinable option to SimpleThread::Options as was just done for Thread.

base/threading/simple_thread.h

 // Copyright (c) 2011 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.
 
 // WARNING: You should probably be using Thread (thread.h) instead.  Thread is
 //          Chrome's message-loop based Thread abstraction, and if you are a
 //          thread running in the browser, there will likely be assumptions
 //          that your thread will have an associated message loop.
 //
 // This is a simple thread interface that backs to a native operating system
@@ skipping 28 matching lines @@
 
 #ifndef BASE_THREADING_SIMPLE_THREAD_H_
 #define BASE_THREADING_SIMPLE_THREAD_H_
 
 #include <stddef.h>
 
 #include <queue>
 #include <string>
 #include <vector>
 
+#include "base/atomic_ref_count.h"
 #include "base/base_export.h"
 #include "base/compiler_specific.h"
+#include "base/logging.h"
+#include "base/macros.h"
 #include "base/synchronization/lock.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/platform_thread.h"
 
 namespace base {
 
 // This is the base SimpleThread.  You can derive from it and implement the
 // virtual Run method, or you can use the DelegateSimpleThread interface.
 class BASE_EXPORT SimpleThread : public PlatformThread::Delegate {
  public:
-  class BASE_EXPORT Options {
+  struct BASE_EXPORT Options {
    public:
-    Options() : stack_size_(0), priority_(ThreadPriority::NORMAL) {}
-    explicit Options(ThreadPriority priority)
-        : stack_size_(0), priority_(priority) {}
-    ~Options() {}
+    Options() = default;
+    explicit Options(ThreadPriority priority_in) : priority(priority_in) {}
+    ~Options() = default;
 
-    // We use the standard compiler-supplied copy constructor.
+    // Allow copies.
+    Options(const Options& other) = default;
+    Options& operator=(const Options& other) = default;
 
     // A custom stack size, or 0 for the system default.
-    void set_stack_size(size_t size) { stack_size_ = size; }
-    size_t stack_size() const { return stack_size_; }
+    size_t stack_size = 0;
 
-    // A custom thread priority.
-    void set_priority(ThreadPriority priority) { priority_ = priority; }
-    ThreadPriority priority() const { return priority_; }
-   private:
-    size_t stack_size_;
-    ThreadPriority priority_;
+    ThreadPriority priority = ThreadPriority::NORMAL;
+
+    // If false, the underlying thread's PlatformThreadHandle will not be kept
+    // around and as such the SimpleThread instance will not be Join()able and
+    // must not be deleted before Run() is invoked. After that, it's up to
+    // the subclass to determine when it is safe to delete itself, see
+    // DelegateSimpleThread for a safe option.
+    bool joinable = true;
   };
 
   // Create a SimpleThread.  |options| should be used to manage any specific
   // configuration involving the thread creation and management.
   // Every thread has a name, in the form of |name_prefix|/TID, for example
   // "my_thread/321".  The thread will not be created until Start() is called.
   explicit SimpleThread(const std::string& name_prefix);
   SimpleThread(const std::string& name_prefix, const Options& options);
 
   ~SimpleThread() override;
 
   virtual void Start();
   virtual void Join();
 
   // Subclasses should override the Run method.
   virtual void Run() = 0;
 
   // Return the thread name prefix, or "unnamed" if none was supplied.
   std::string name_prefix() { return name_prefix_; }
 
   // Return the completed name including TID, only valid after Start().
   std::string name() { return name_; }
 
   // Return the thread id, only valid after Start().
   PlatformThreadId tid() { return tid_; }
 
   // Return True if Start() has ever been called.
   bool HasBeenStarted();
 
-  // Return True if Join() has evern been called.
+  // Return True if Join() has ever been called.
   bool HasBeenJoined() { return joined_; }
 
   // Overridden from PlatformThread::Delegate:
   void ThreadMain() override;
 
  private:
   const std::string name_prefix_;
   std::string name_;
   const Options options_;
-  PlatformThreadHandle thread_;  // PlatformThread handle, invalid after Join!
+  PlatformThreadHandle thread_;  // PlatformThread handle, reset after Join.
   WaitableEvent event_;          // Signaled if Start() was ever called.
-  PlatformThreadId tid_;         // The backing thread's id.
-  bool joined_;                  // True if Join has been called.
+  PlatformThreadId tid_ = kInvalidThreadId;  // The backing thread's id.
+  bool joined_ = false;                      // True if Join has been called.
+
+  DISALLOW_COPY_AND_ASSIGN(SimpleThread);
 };
 
+// A SimpleThread which delegates Run() to its Delegate. The registered Delegate
+// must outlive its Run() invocation(s). As such, non-joinable
+// DelegateSimpleThread are safe to delete after Start() so long as the
+// Delegate's lifetime respects the aforementioned condition.
 class BASE_EXPORT DelegateSimpleThread : public SimpleThread {
  public:
   class BASE_EXPORT Delegate {
    public:
-    Delegate() { }
-    virtual ~Delegate() { }
+    Delegate();
+    virtual ~Delegate();
+
+    // Invokes Run() and manages |active_runners_|.
+    void RunHelper();
+
+   private:
+    // Run() is private pure virtual because only RunHelper() should invoke it,

[danakj, 2016/08/12 18:38:00]

but.... you can just upcast it and call Run() directly tho (even accidentally).

[gab, 2016/08/12 18:45:04]

You mean because the impl can override it as public? I guess but someone
explicitly calling run on a Delegate would be obviously wrong IMO -- even
RunHelper is only intended to be called by DelegateSimpleThread.

This is mostly hinting people further to do the right thing. Even if someone
checks in something terribly wrong, worst case is just that the DCHECKs are
skipped (provided they got everything else right).

[danakj, 2016/08/12 18:55:17]

I mean like

SimpleThread* thread = new DelegateSimpleThread;
thread->Run();

[gab, 2016/08/12 19:05:49]

This is different. The code this comment is on is Delegate::Run().
DelegateSimpleThread::Run() is on line 171. Calling Run() from the owner is
always wrong it's intended to be called by ThreadMain (both for
DelegateSimpleThread and SimpleThread), this CL doesn't change that.

[danakj, 2016/08/12 20:49:10]

Oooh, nested class, I see. rip reading

+    // but it's still up to the implementer to define it.
     virtual void Run() = 0;
+
+#if DCHECK_IS_ON()
+    // Maintains a count of active Run() calls -- there can be more than one
+    // when Delegate is used in a DelegateSimpleThreadPool.
+    AtomicRefCount active_runners_ = 0;
+
+    // Signaled when |active_runners_| is decremented, auto-reset after the
+    // signal is observed.
+    WaitableEvent active_runners_decremented_;
+#endif  // DCHECK_IS_ON()
+
+    DISALLOW_COPY_AND_ASSIGN(Delegate);
   };
 
   DelegateSimpleThread(Delegate* delegate,
                        const std::string& name_prefix);
   DelegateSimpleThread(Delegate* delegate,
                        const std::string& name_prefix,
                        const Options& options);
 
   ~DelegateSimpleThread() override;
   void Run() override;
 
  private:
   Delegate* delegate_;
+
+#if DCHECK_IS_ON()
+  bool ran_ = false;
+#endif
+
+  DISALLOW_COPY_AND_ASSIGN(DelegateSimpleThread);
 };
 
 // DelegateSimpleThreadPool allows you to start up a fixed number of threads,
 // and then add jobs which will be dispatched to the threads.  This is
 // convenient when you have a lot of small work that you want done
 // multi-threaded, but don't want to spawn a thread for each small bit of work.
 //
 // You just call AddWork() to add a delegate to the list of work to be done.
 // JoinAll() will make sure that all outstanding work is processed, and wait
 // for everything to finish.  You can reuse a pool, so you can call Start()
@@ skipping 24 matching lines @@
   // We implement the Delegate interface, for running our internal threads.
   void Run() override;
 
  private:
   const std::string name_prefix_;
   int num_threads_;
   std::vector<DelegateSimpleThread*> threads_;
   std::queue<Delegate*> delegates_;
   base::Lock lock_;            // Locks delegates_
   WaitableEvent dry_;    // Not signaled when there is no work to do.
+
+  DISALLOW_COPY_AND_ASSIGN(DelegateSimpleThreadPool);
 };
 
 }  // namespace base
 
 #endif  // BASE_THREADING_SIMPLE_THREAD_H_