Commit b6c96e72 authored by Gabriel Charette's avatar Gabriel Charette Committed by Commit Bot

[Tracing] Remove base::Thread usage from ETWTracingAgent

post_task.h is now preferred to creating special purposed base::Threads

All this code needs is a SequencedTaskRunner which is allowed to
perform file I/O.

Furthermore, PlatformThread::Join() and hence Thread::Stop() will be
marked as a blocking operation in
https://chromium-review.googlesource.com/c/chromium/src/+/1324370
and this code was preventing that (even though in this case the
Join() was mostly a fast operation as there were no tasks left by the
time it was invoked).

R=chiniforooshan@chromium.org

Bug: 707362
Change-Id: Iec2f823e762cea2d1d1e775117fd95d871baa574
Reviewed-on: https://chromium-review.googlesource.com/c/1338178Reviewed-by: default avatarEhsan Chiniforooshan <chiniforooshan@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608610}
parent 4a3e441a
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include <vector> #include <vector>
#include "base/macros.h" #include "base/macros.h"
#include "base/threading/scoped_blocking_call.h"
namespace base { namespace base {
namespace win { namespace win {
...@@ -120,6 +121,7 @@ HRESULT EtwTraceConsumerBase<ImplClass>::OpenFileSession( ...@@ -120,6 +121,7 @@ HRESULT EtwTraceConsumerBase<ImplClass>::OpenFileSession(
template <class ImplClass> inline template <class ImplClass> inline
HRESULT EtwTraceConsumerBase<ImplClass>::Consume() { HRESULT EtwTraceConsumerBase<ImplClass>::Consume() {
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
ULONG err = ::ProcessTrace(&trace_handles_[0], ULONG err = ::ProcessTrace(&trace_handles_[0],
static_cast<ULONG>(trace_handles_.size()), static_cast<ULONG>(trace_handles_.size()),
NULL, NULL,
......
...@@ -50,7 +50,8 @@ EtwTracingAgent::EtwTracingAgent(service_manager::Connector* connector) ...@@ -50,7 +50,8 @@ EtwTracingAgent::EtwTracingAgent(service_manager::Connector* connector)
tracing::mojom::TraceDataType::OBJECT, tracing::mojom::TraceDataType::OBJECT,
false /* supports_explicit_clock_sync */, false /* supports_explicit_clock_sync */,
base::kNullProcessId), base::kNullProcessId),
thread_("EtwConsumerThread"), tracing_task_runner_(base::CreateSequencedTaskRunnerWithTraits(
{base::MayBlock(), base::TaskPriority::USER_VISIBLE})),
is_tracing_(false) { is_tracing_(false) {
DCHECK(!g_etw_tracing_agent); DCHECK(!g_etw_tracing_agent);
g_etw_tracing_agent = this; g_etw_tracing_agent = this;
...@@ -73,14 +74,12 @@ void EtwTracingAgent::StartTracing(const std::string& config, ...@@ -73,14 +74,12 @@ void EtwTracingAgent::StartTracing(const std::string& config,
} }
is_tracing_ = true; is_tracing_ = true;
// Start the consumer thread and start consuming events.
thread_.Start();
// Tracing agents, e.g. this, live as long as BrowserMainLoop lives and so // Tracing agents, e.g. this, live as long as BrowserMainLoop lives and so
// using base::Unretained here is safe. // using base::Unretained here is safe.
thread_.task_runner()->PostTask( tracing_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&EtwTracingAgent::TraceAndConsumeOnThread, FROM_HERE,
base::Unretained(this))); base::BindOnce(&EtwTracingAgent::TraceAndConsumeOnTracingSequence,
base::Unretained(this)));
std::move(callback).Run(true /* success */); std::move(callback).Run(true /* success */);
} }
...@@ -93,16 +92,14 @@ void EtwTracingAgent::StopAndFlush(tracing::mojom::RecorderPtr recorder) { ...@@ -93,16 +92,14 @@ void EtwTracingAgent::StopAndFlush(tracing::mojom::RecorderPtr recorder) {
recorder_ = std::move(recorder); recorder_ = std::move(recorder);
// Stop consuming and flush events. Tracing agents, e.g. this, live as long as // Stop consuming and flush events. Tracing agents, e.g. this, live as long as
// BrowserMainLoop lives and so using base::Unretained here is safe. // BrowserMainLoop lives and so using base::Unretained here is safe.
thread_.task_runner()->PostTask( tracing_task_runner_->PostTask(
FROM_HERE, FROM_HERE, base::BindOnce(&EtwTracingAgent::FlushOnTracingSequence,
base::BindOnce(&EtwTracingAgent::FlushOnThread, base::Unretained(this))); base::Unretained(this)));
} }
void EtwTracingAgent::OnStopSystemTracingDone(const std::string& output) { void EtwTracingAgent::OnStopSystemTracingDone(const std::string& output) {
recorder_->AddChunk(output); recorder_->AddChunk(output);
recorder_.reset(); recorder_.reset();
// Stop the consumer thread.
thread_.Stop();
is_tracing_ = false; is_tracing_ = false;
} }
...@@ -217,7 +214,9 @@ void EtwTracingAgent::AppendEventToBuffer(EVENT_TRACE* event) { ...@@ -217,7 +214,9 @@ void EtwTracingAgent::AppendEventToBuffer(EVENT_TRACE* event) {
events_->Append(std::move(value)); events_->Append(std::move(value));
} }
void EtwTracingAgent::TraceAndConsumeOnThread() { void EtwTracingAgent::TraceAndConsumeOnTracingSequence() {
DCHECK(tracing_task_runner_->RunsTasksInCurrentSequence());
// Create the events buffer. // Create the events buffer.
events_.reset(new base::ListValue()); events_.reset(new base::ListValue());
...@@ -231,7 +230,9 @@ void EtwTracingAgent::TraceAndConsumeOnThread() { ...@@ -231,7 +230,9 @@ void EtwTracingAgent::TraceAndConsumeOnThread() {
Close(); Close();
} }
void EtwTracingAgent::FlushOnThread() { void EtwTracingAgent::FlushOnTracingSequence() {
DCHECK(tracing_task_runner_->RunsTasksInCurrentSequence());
// Add the header information to the stream. // Add the header information to the stream.
auto header = std::make_unique<base::DictionaryValue>(); auto header = std::make_unique<base::DictionaryValue>();
header->SetString("name", "ETW"); header->SetString("name", "ETW");
......
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ref_counted_memory.h" #include "base/memory/ref_counted_memory.h"
#include "base/threading/thread.h" #include "base/sequenced_task_runner.h"
#include "base/values.h" #include "base/values.h"
#include "base/win/event_trace_consumer.h" #include "base/win/event_trace_consumer.h"
#include "base/win/event_trace_controller.h" #include "base/win/event_trace_controller.h"
...@@ -61,11 +61,13 @@ class EtwTracingAgent : public base::win::EtwTraceConsumerBase<EtwTracingAgent>, ...@@ -61,11 +61,13 @@ class EtwTracingAgent : public base::win::EtwTraceConsumerBase<EtwTracingAgent>,
bool StopKernelSessionTracing(); bool StopKernelSessionTracing();
void OnStopSystemTracingDone(const std::string& output); void OnStopSystemTracingDone(const std::string& output);
void TraceAndConsumeOnThread(); void TraceAndConsumeOnTracingSequence();
void FlushOnThread(); void FlushOnTracingSequence();
// Task runner used to communicate with the native tracing interface.
scoped_refptr<base::SequencedTaskRunner> tracing_task_runner_;
std::unique_ptr<base::ListValue> events_; std::unique_ptr<base::ListValue> events_;
base::Thread thread_;
TRACEHANDLE session_handle_; TRACEHANDLE session_handle_;
base::win::EtwTraceProperties properties_; base::win::EtwTraceProperties properties_;
tracing::mojom::RecorderPtr recorder_; tracing::mojom::RecorderPtr recorder_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment