Create and remove |UiDevToolsServer::server_| in the same thread.

components/ui_devtools/devtools_server.cc

Index: components/ui_devtools/devtools_server.cc
diff --git a/components/ui_devtools/devtools_server.cc b/components/ui_devtools/devtools_server.cc
index c8fd76c15f1bbb4aa29ed94ce537c75446a43eb1..aceecaa75e3d4d88b9cf5a6968f1d71d83ee0fac 100644
--- a/components/ui_devtools/devtools_server.cc
+++ b/components/ui_devtools/devtools_server.cc
@@ -65,6 +65,12 @@ UiDevToolsServer::UiDevToolsServer(
 }
 
 UiDevToolsServer::~UiDevToolsServer() {
+  DCHECK(io_thread_task_runner_);
+  io_thread_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&UiDevToolsServer::StopServer, base::Unretained(this)));
+  if (thread_->IsRunning())

[sadrul, 2017/06/26 22:43:30]

This will not work when |io_thread_task_runner_| has been injected from outside
during creation. For that case, you would either need to use a WaitableEvent
(see examples of that in various places), or do something like this:

  io_thread_task_runner_->DeleteSoon(FROM_HERE, server_.release());

[thanhph, 2017/06/27 15:02:29]

To elaborate more, with current patch 1, does this mean we are afraid
|io_thread_task_runner_| gets cleaned up or gets blocked before
|io_thread_task_runner_| is used here? PostTask() uses PostDelayedTask() which
has default timedelta = 0, thus invoking StopSever() in future time with no
delay (but I guess still could get blocked with current running tasks) and
DeleteSoon() uses PostNonNestableTask() which provides this task will not run
nested within an already-running task.

[sadrul, 2017/06/28 04:49:10]

No. The problem with this change was, that if UiDevToolsServer() received a
custom io_thread_task_runner during creation, then it does not create |thread_|.
Which means this UiDevToolsServer object can still get destroyed before the task
you have just posted runs. This means (1) |server_| gets immediately destroyed,
still in the wrong thread, and on top of that (2) the posted task does run on
the io thread, but |this| has already been destroyed, leading to a
use-after-free.

The most important part of the approach with DeleteSoon() is that you give up
ownership of |server_|. Which means |this| can get destroyed, without destroying
the server. And the server will be correctly destroyed on the IO thread.

If the IO thread has already been torn down, that means |server_| will just
never be destroyed, and will leak instead. But I don't think there's a clean way
of fixing that.

[thanhph, 2017/06/28 14:38:22]

Acknowledged! Thanks for the explanation.

+    thread_->Stop();
   devtools_server_ = nullptr;
 }
 
@@ -127,6 +133,10 @@ void UiDevToolsServer::StartServer(const std::string& address_string,
   server_ = base::MakeUnique<net::HttpServer>(std::move(socket), this);
 }
 
+void UiDevToolsServer::StopServer() {
+  server_.reset();
+}
+
 // HttpServer::Delegate Implementation
 void UiDevToolsServer::OnConnect(int connection_id) {
   NOTIMPLEMENTED();