Commit 7f1f6062 authored by Etienne Pierre-Doray's avatar Etienne Pierre-Doray Committed by Commit Bot

[TaskScheduler]: Use ScopedBlockingCall to mark blocking tasks.

This CL uses ScopedBlockingCall to mark blocking calls in /dbus.

This CL adds ScopedBlockingCall(MAY_BLOCK) for each scope containing a dbus_*
call, assuming all of those calls may block.
I kindly ask the reviewer to make sure of the following:
  - ScopedBlockingCall is instantiated in a scope with minimal CPU usage.
    If this is not the case, ScopedBlockingCall should be instantiated
    closer to the blocking call. See scoped_blocking_call.h for more
    info. Please let me know when/where the blocking call happens if this needs
    to be changed.
  - Parameter |blocking_type| matches expectation (MAY_BLOCK/WILL_BLOCK). See
    BlockingType for more info. While I assumed MAY_BLOCK by default, that might
    not be the best fit if we know that this callsite is guaranteed to block.
  - The ScopedBlockingCall's scope covers the entirety of the blocking operation
    previously asserted against by the AssertBlockingAllowed().

This CL was uploaded by git cl split.

R=satorux@chromium.org

Bug: 874080
Change-Id: Ib90b59280bd22cb671a64bacabe299c02c6d6081
Reviewed-on: https://chromium-review.googlesource.com/1190930Reviewed-by: default avatarRyo Hashimoto <hashimoto@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593177}
parent ccb95b1f
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/stl_util.h" #include "base/stl_util.h"
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/threading/scoped_blocking_call.h"
#include "base/threading/thread.h" #include "base/threading/thread.h"
#include "base/threading/thread_restrictions.h" #include "base/threading/thread_restrictions.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
...@@ -370,6 +371,7 @@ void Bus::RemoveObjectManagerInternalHelper( ...@@ -370,6 +371,7 @@ void Bus::RemoveObjectManagerInternalHelper(
bool Bus::Connect() { bool Bus::Connect() {
// dbus_bus_get_private() and dbus_bus_get() are blocking calls. // dbus_bus_get_private() and dbus_bus_get() are blocking calls.
AssertOnDBusThread(); AssertOnDBusThread();
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
// Check if it's already initialized. // Check if it's already initialized.
if (connection_) if (connection_)
...@@ -423,6 +425,7 @@ void Bus::ClosePrivateConnection() { ...@@ -423,6 +425,7 @@ void Bus::ClosePrivateConnection() {
AssertOnDBusThread(); AssertOnDBusThread();
DCHECK_EQ(PRIVATE, connection_type_) DCHECK_EQ(PRIVATE, connection_type_)
<< "non-private connection should not be closed"; << "non-private connection should not be closed";
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
dbus_connection_close(connection_); dbus_connection_close(connection_);
} }
...@@ -471,6 +474,9 @@ void Bus::ShutdownAndBlock() { ...@@ -471,6 +474,9 @@ void Bus::ShutdownAndBlock() {
// Private connection should be closed. // Private connection should be closed.
if (connection_) { if (connection_) {
base::ScopedBlockingCall scoped_blocking_call(
base::BlockingType::MAY_BLOCK);
// Remove Disconnected watcher. // Remove Disconnected watcher.
ScopedDBusError error; ScopedDBusError error;
RemoveFilterFunction(Bus::OnConnectionDisconnectedFilter, this); RemoveFilterFunction(Bus::OnConnectionDisconnectedFilter, this);
...@@ -542,6 +548,7 @@ bool Bus::RequestOwnershipAndBlock(const std::string& service_name, ...@@ -542,6 +548,7 @@ bool Bus::RequestOwnershipAndBlock(const std::string& service_name,
return true; return true;
} }
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
ScopedDBusError error; ScopedDBusError error;
const int result = dbus_bus_request_name(connection_, const int result = dbus_bus_request_name(connection_,
service_name.c_str(), service_name.c_str(),
...@@ -558,7 +565,7 @@ bool Bus::RequestOwnershipAndBlock(const std::string& service_name, ...@@ -558,7 +565,7 @@ bool Bus::RequestOwnershipAndBlock(const std::string& service_name,
bool Bus::ReleaseOwnership(const std::string& service_name) { bool Bus::ReleaseOwnership(const std::string& service_name) {
DCHECK(connection_); DCHECK(connection_);
// dbus_bus_request_name() is a blocking call. // dbus_bus_release_name() is a blocking call.
AssertOnDBusThread(); AssertOnDBusThread();
// Check if we already own the service name. // Check if we already own the service name.
...@@ -569,6 +576,7 @@ bool Bus::ReleaseOwnership(const std::string& service_name) { ...@@ -569,6 +576,7 @@ bool Bus::ReleaseOwnership(const std::string& service_name) {
return false; return false;
} }
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
ScopedDBusError error; ScopedDBusError error;
const int result = dbus_bus_release_name(connection_, service_name.c_str(), const int result = dbus_bus_release_name(connection_, service_name.c_str(),
error.get()); error.get());
...@@ -594,6 +602,7 @@ bool Bus::SetUpAsyncOperations() { ...@@ -594,6 +602,7 @@ bool Bus::SetUpAsyncOperations() {
// be called when the incoming data is ready. // be called when the incoming data is ready.
ProcessAllIncomingDataIfAny(); ProcessAllIncomingDataIfAny();
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
bool success = dbus_connection_set_watch_functions( bool success = dbus_connection_set_watch_functions(
connection_, &Bus::OnAddWatchThunk, &Bus::OnRemoveWatchThunk, connection_, &Bus::OnAddWatchThunk, &Bus::OnRemoveWatchThunk,
&Bus::OnToggleWatchThunk, this, nullptr); &Bus::OnToggleWatchThunk, this, nullptr);
...@@ -618,6 +627,7 @@ DBusMessage* Bus::SendWithReplyAndBlock(DBusMessage* request, ...@@ -618,6 +627,7 @@ DBusMessage* Bus::SendWithReplyAndBlock(DBusMessage* request,
DCHECK(connection_); DCHECK(connection_);
AssertOnDBusThread(); AssertOnDBusThread();
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
return dbus_connection_send_with_reply_and_block( return dbus_connection_send_with_reply_and_block(
connection_, request, timeout_ms, error); connection_, request, timeout_ms, error);
} }
...@@ -628,6 +638,7 @@ void Bus::SendWithReply(DBusMessage* request, ...@@ -628,6 +638,7 @@ void Bus::SendWithReply(DBusMessage* request,
DCHECK(connection_); DCHECK(connection_);
AssertOnDBusThread(); AssertOnDBusThread();
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
const bool success = dbus_connection_send_with_reply( const bool success = dbus_connection_send_with_reply(
connection_, request, pending_call, timeout_ms); connection_, request, pending_call, timeout_ms);
CHECK(success) << "Unable to allocate memory"; CHECK(success) << "Unable to allocate memory";
...@@ -637,6 +648,7 @@ void Bus::Send(DBusMessage* request, uint32_t* serial) { ...@@ -637,6 +648,7 @@ void Bus::Send(DBusMessage* request, uint32_t* serial) {
DCHECK(connection_); DCHECK(connection_);
AssertOnDBusThread(); AssertOnDBusThread();
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
const bool success = dbus_connection_send(connection_, request, serial); const bool success = dbus_connection_send(connection_, request, serial);
CHECK(success) << "Unable to allocate memory"; CHECK(success) << "Unable to allocate memory";
} }
...@@ -655,6 +667,7 @@ void Bus::AddFilterFunction(DBusHandleMessageFunction filter_function, ...@@ -655,6 +667,7 @@ void Bus::AddFilterFunction(DBusHandleMessageFunction filter_function,
return; return;
} }
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
const bool success = dbus_connection_add_filter(connection_, filter_function, const bool success = dbus_connection_add_filter(connection_, filter_function,
user_data, nullptr); user_data, nullptr);
CHECK(success) << "Unable to allocate memory"; CHECK(success) << "Unable to allocate memory";
...@@ -676,6 +689,7 @@ void Bus::RemoveFilterFunction(DBusHandleMessageFunction filter_function, ...@@ -676,6 +689,7 @@ void Bus::RemoveFilterFunction(DBusHandleMessageFunction filter_function,
return; return;
} }
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
dbus_connection_remove_filter(connection_, filter_function, user_data); dbus_connection_remove_filter(connection_, filter_function, user_data);
filter_functions_added_.erase(filter_data_pair); filter_functions_added_.erase(filter_data_pair);
} }
...@@ -694,6 +708,7 @@ void Bus::AddMatch(const std::string& match_rule, DBusError* error) { ...@@ -694,6 +708,7 @@ void Bus::AddMatch(const std::string& match_rule, DBusError* error) {
return; return;
} }
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
dbus_bus_add_match(connection_, match_rule.c_str(), error); dbus_bus_add_match(connection_, match_rule.c_str(), error);
match_rules_added_[match_rule] = 1; match_rules_added_[match_rule] = 1;
} }
...@@ -709,6 +724,7 @@ bool Bus::RemoveMatch(const std::string& match_rule, DBusError* error) { ...@@ -709,6 +724,7 @@ bool Bus::RemoveMatch(const std::string& match_rule, DBusError* error) {
return false; return false;
} }
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
// The rule's counter is decremented and the rule is deleted when reachs 0. // The rule's counter is decremented and the rule is deleted when reachs 0.
iter->second--; iter->second--;
if (iter->second == 0) { if (iter->second == 0) {
...@@ -743,6 +759,7 @@ bool Bus::TryRegisterObjectPathInternal( ...@@ -743,6 +759,7 @@ bool Bus::TryRegisterObjectPathInternal(
TryRegisterObjectPathFunction* register_function) { TryRegisterObjectPathFunction* register_function) {
DCHECK(connection_); DCHECK(connection_);
AssertOnDBusThread(); AssertOnDBusThread();
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
if (registered_object_paths_.find(object_path) != if (registered_object_paths_.find(object_path) !=
registered_object_paths_.end()) { registered_object_paths_.end()) {
...@@ -768,6 +785,7 @@ void Bus::UnregisterObjectPath(const ObjectPath& object_path) { ...@@ -768,6 +785,7 @@ void Bus::UnregisterObjectPath(const ObjectPath& object_path) {
return; return;
} }
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
const bool success = dbus_connection_unregister_object_path( const bool success = dbus_connection_unregister_object_path(
connection_, connection_,
object_path.value().c_str()); object_path.value().c_str());
...@@ -789,6 +807,8 @@ void Bus::ProcessAllIncomingDataIfAny() { ...@@ -789,6 +807,8 @@ void Bus::ProcessAllIncomingDataIfAny() {
if (!connection_) if (!connection_)
return; return;
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
// It is safe and necessary to call dbus_connection_get_dispatch_status even // It is safe and necessary to call dbus_connection_get_dispatch_status even
// if the connection is lost. // if the connection is lost.
if (dbus_connection_get_dispatch_status(connection_) == if (dbus_connection_get_dispatch_status(connection_) ==
...@@ -820,8 +840,6 @@ void Bus::AssertOnOriginThread() { ...@@ -820,8 +840,6 @@ void Bus::AssertOnOriginThread() {
} }
void Bus::AssertOnDBusThread() { void Bus::AssertOnDBusThread() {
base::AssertBlockingAllowed();
if (dbus_task_runner_) { if (dbus_task_runner_) {
DCHECK(dbus_task_runner_->RunsTasksInCurrentSequence()); DCHECK(dbus_task_runner_->RunsTasksInCurrentSequence());
} else { } else {
...@@ -1016,6 +1034,7 @@ dbus_bool_t Bus::OnAddWatch(DBusWatch* raw_watch) { ...@@ -1016,6 +1034,7 @@ dbus_bool_t Bus::OnAddWatch(DBusWatch* raw_watch) {
void Bus::OnRemoveWatch(DBusWatch* raw_watch) { void Bus::OnRemoveWatch(DBusWatch* raw_watch) {
AssertOnDBusThread(); AssertOnDBusThread();
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
Watch* watch = static_cast<Watch*>(dbus_watch_get_data(raw_watch)); Watch* watch = static_cast<Watch*>(dbus_watch_get_data(raw_watch));
delete watch; delete watch;
--num_pending_watches_; --num_pending_watches_;
...@@ -1024,6 +1043,7 @@ void Bus::OnRemoveWatch(DBusWatch* raw_watch) { ...@@ -1024,6 +1043,7 @@ void Bus::OnRemoveWatch(DBusWatch* raw_watch) {
void Bus::OnToggleWatch(DBusWatch* raw_watch) { void Bus::OnToggleWatch(DBusWatch* raw_watch) {
AssertOnDBusThread(); AssertOnDBusThread();
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
Watch* watch = static_cast<Watch*>(dbus_watch_get_data(raw_watch)); Watch* watch = static_cast<Watch*>(dbus_watch_get_data(raw_watch));
if (watch->IsReadyToBeWatched()) if (watch->IsReadyToBeWatched())
watch->StartWatching(); watch->StartWatching();
...@@ -1046,6 +1066,7 @@ dbus_bool_t Bus::OnAddTimeout(DBusTimeout* raw_timeout) { ...@@ -1046,6 +1066,7 @@ dbus_bool_t Bus::OnAddTimeout(DBusTimeout* raw_timeout) {
void Bus::OnRemoveTimeout(DBusTimeout* raw_timeout) { void Bus::OnRemoveTimeout(DBusTimeout* raw_timeout) {
AssertOnDBusThread(); AssertOnDBusThread();
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
Timeout* timeout = static_cast<Timeout*>(dbus_timeout_get_data(raw_timeout)); Timeout* timeout = static_cast<Timeout*>(dbus_timeout_get_data(raw_timeout));
delete timeout; delete timeout;
--num_pending_timeouts_; --num_pending_timeouts_;
...@@ -1054,6 +1075,7 @@ void Bus::OnRemoveTimeout(DBusTimeout* raw_timeout) { ...@@ -1054,6 +1075,7 @@ void Bus::OnRemoveTimeout(DBusTimeout* raw_timeout) {
void Bus::OnToggleTimeout(DBusTimeout* raw_timeout) { void Bus::OnToggleTimeout(DBusTimeout* raw_timeout) {
AssertOnDBusThread(); AssertOnDBusThread();
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
Timeout* timeout = static_cast<Timeout*>(dbus_timeout_get_data(raw_timeout)); Timeout* timeout = static_cast<Timeout*>(dbus_timeout_get_data(raw_timeout));
if (timeout->IsReadyToBeMonitored()) { if (timeout->IsReadyToBeMonitored()) {
timeout->StartMonitoring(this); timeout->StartMonitoring(this);
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/task_runner.h" #include "base/task_runner.h"
#include "base/task_runner_util.h" #include "base/task_runner_util.h"
#include "base/threading/scoped_blocking_call.h"
#include "base/threading/thread.h" #include "base/threading/thread.h"
#include "base/threading/thread_restrictions.h" #include "base/threading/thread_restrictions.h"
#include "dbus/bus.h" #include "dbus/bus.h"
...@@ -302,6 +303,9 @@ void ObjectProxy::Detach() { ...@@ -302,6 +303,9 @@ void ObjectProxy::Detach() {
match_rules_.clear(); match_rules_.clear();
for (auto* pending_call : pending_calls_) { for (auto* pending_call : pending_calls_) {
base::ScopedBlockingCall scoped_blocking_call(
base::BlockingType::MAY_BLOCK);
dbus_pending_call_cancel(pending_call); dbus_pending_call_cancel(pending_call);
dbus_pending_call_unref(pending_call); dbus_pending_call_unref(pending_call);
} }
...@@ -313,6 +317,7 @@ void ObjectProxy::StartAsyncMethodCall(int timeout_ms, ...@@ -313,6 +317,7 @@ void ObjectProxy::StartAsyncMethodCall(int timeout_ms,
ReplyCallbackHolder callback_holder, ReplyCallbackHolder callback_holder,
base::TimeTicks start_time) { base::TimeTicks start_time) {
bus_->AssertOnDBusThread(); bus_->AssertOnDBusThread();
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
if (!bus_->Connect() || !bus_->SetUpAsyncOperations()) { if (!bus_->Connect() || !bus_->SetUpAsyncOperations()) {
// In case of a failure, run the error callback with nullptr. // In case of a failure, run the error callback with nullptr.
...@@ -353,6 +358,7 @@ void ObjectProxy::OnPendingCallIsComplete(ReplyCallbackHolder callback_holder, ...@@ -353,6 +358,7 @@ void ObjectProxy::OnPendingCallIsComplete(ReplyCallbackHolder callback_holder,
base::TimeTicks start_time, base::TimeTicks start_time,
DBusPendingCall* pending_call) { DBusPendingCall* pending_call) {
bus_->AssertOnDBusThread(); bus_->AssertOnDBusThread();
base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
DBusMessage* response_message = dbus_pending_call_steal_reply(pending_call); DBusMessage* response_message = dbus_pending_call_steal_reply(pending_call);
......
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