Tracing: active tracing at shutdown

If the reader of the stream stops reading from it for some
reason, a deadlock is possible:

- WriteToStream() obtains stream_lock_ but does not release
  it because it is blocked on writing to the stream that is
  not read from.

- CloseStream() does not close the stream because it waits
  on the lock that WriteToStream() is not releasing.

I will remove the lock alltogether. So, CloseStream() does
not wait and just closes the stream. Then, if the
BlockingCopyFromString() statement of WriteToStream() is
blocked, it will be signaled that the stream is closed.

It is not a perfect solution because if CloseStream() closes
the stream after the "if (stream_.is_valid())" check of
WriteToStream() but before BlockingCopyFromString() reaches
the wait statement, the stream handle becomes invalid and a
DCHECK will fail.

I cannot think of a simple solution right now, but the
above-mentioned scenario is very unlikely to happen. For
sure, this CL improves the current flakiness.

Bug: 837215
Change-Id: I5d064b93dfb6af444c778bb60f156b29b43f1617
Reviewed-on: https://chromium-review.googlesource.com/1054092
Commit-Queue: Ehsan Chiniforooshan <chiniforooshan@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557677}

services/tracing/coordinator.cc

@@ -19,7 +19,6 @@
 #include "base/logging.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
-#include "base/synchronization/lock.h"
 #include "base/task_scheduler/post_task.h"
 #include "base/task_scheduler/task_traits.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -98,7 +97,6 @@ class Coordinator::TraceStreamer : public base::SupportsWeakPtr<TraceStreamer> {
   // shutdown. We either will not write to the stream afterwards or do not care
   // what happens to what we try to write.
   void CloseStream() {
-    base::AutoLock mutex(stream_lock_);
     DCHECK(stream_.is_valid());
     stream_.reset();
   }
@@ -113,8 +111,7 @@ class Coordinator::TraceStreamer : public base::SupportsWeakPtr<TraceStreamer> {
   // Handles synchronize writes to |stream_|, if the stream is not already
   // closed.
   void WriteToStream(const std::string& data) {
-    base::AutoLock mutex(stream_lock_);
-    if (stream_->is_valid())
+    if (stream_.is_valid())
       mojo::BlockingCopyFromString(data, stream_);
   }
 
@@ -257,7 +254,6 @@ class Coordinator::TraceStreamer : public base::SupportsWeakPtr<TraceStreamer> {
   // The stream to which trace events from different agents should be
   // serialized, eventually. This is set when tracing is stopped.
   mojo::ScopedDataPipeProducerHandle stream_;
-  base::Lock stream_lock_;
   std::string agent_label_;
   scoped_refptr<base::SequencedTaskRunner> main_task_runner_;
   base::WeakPtr<Coordinator> coordinator_;