Skip to content

Commit

Permalink
Convert from using SingleThreadTaskRunner to SequencedTaskRunner (#245)
Browse files Browse the repository at this point in the history
b/243166974
  • Loading branch information
sherryzy committed May 15, 2023
1 parent 8b54d05 commit a16f613
Show file tree
Hide file tree
Showing 29 changed files with 151 additions and 97 deletions.
36 changes: 36 additions & 0 deletions base/sequenced_task_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,48 @@
// found in the LICENSE file.

#include "base/sequenced_task_runner.h"
#include "base/message_loop/message_loop.h"
#include "base/synchronization/waitable_event.h"
#include "base/trace_event/trace_event.h"

#include <utility>

#include "base/bind.h"

namespace base {
#if defined(STARBOARD)
namespace {

// Runs the given task, and then signals the given WaitableEvent.
void RunAndSignal(const base::Closure& task, base::WaitableEvent* event) {
TRACE_EVENT0("task", "RunAndSignal");
task.Run();
event->Signal();
}
} // namespace

void SequencedTaskRunner::PostBlockingTask(const base::Location& from_here,
const base::Closure& task) {
TRACE_EVENT0("task", "MessageLoop::PostBlockingTask");
DCHECK(!RunsTasksInCurrentSequence())
<< "PostBlockingTask can't be called from the MessageLoop's own thread. "
<< from_here.ToString();
DCHECK(!task.is_null()) << from_here.ToString();

base::WaitableEvent task_finished(
base::WaitableEvent::ResetPolicy::MANUAL,
base::WaitableEvent::InitialState::NOT_SIGNALED);
bool task_may_run = PostTask(from_here,
base::Bind(&RunAndSignal, task, base::Unretained(&task_finished)));
DCHECK(task_may_run)
<< "Task that will never run posted with PostBlockingTask.";

if (task_may_run) {
// Wait for the task to complete before proceeding.
task_finished.Wait();
}
}
#endif

bool SequencedTaskRunner::PostNonNestableTask(const Location& from_here,
OnceClosure task) {
Expand Down
20 changes: 20 additions & 0 deletions base/sequenced_task_runner.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <memory>

#include "base/base_export.h"
#include "base/bind.h"
#include "base/callback.h"
#include "base/sequenced_task_runner_helpers.h"
#include "base/task_runner.h"
Expand Down Expand Up @@ -140,6 +141,25 @@ class BASE_EXPORT SequencedTaskRunner : public TaskRunner {
object);
}

#if defined(STARBOARD)
// Like PostTask, but blocks until the posted task completes. Returns false
// and does not block if task was not posted.
virtual void PostBlockingTask(const base::Location& from_here,
const Closure& task);

// Adds a fence at the end of this MessageLoop's task queue, and then blocks
// until it has been reached. It is forbidden to call this method from the
// thread of the MessageLoop being posted to. One should exercise extreme
// caution when using this, as blocking between MessageLoops can cause
// deadlocks and is contraindicated in the Actor model of multiprogramming.
void WaitForFence() {
struct Fence {
static void Task() {}
};
PostBlockingTask(FROM_HERE, base::Bind(&Fence::Task));
}
#endif

protected:
~SequencedTaskRunner() override = default;

Expand Down
2 changes: 1 addition & 1 deletion base/single_thread_task_runner.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class BASE_EXPORT SingleThreadTaskRunner : public SequencedTaskRunner {
// Like PostTask, but blocks until the posted task completes. Returns false
// and does not block if task was not posted.
virtual void PostBlockingTask(const base::Location& from_here,
const Closure& task);
const Closure& task) override;

// Adds a fence at the end of this MessageLoop's task queue, and then blocks
// until it has been reached. It is forbidden to call this method from the
Expand Down
2 changes: 1 addition & 1 deletion cobalt/h5vcc/h5vcc_storage.cc
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ void H5vccStorage::ClearCookies() {
net::CookieStore* cookie_store =
network_module_->url_request_context()->cookie_store();
auto* cookie_monster = static_cast<net::CookieMonster*>(cookie_store);
network_module_->task_runner()->PostBlockingTask(
network_module_->task_runner()->PostTask(
FROM_HERE,
base::Bind(&net::CookieMonster::DeleteAllMatchingInfoAsync,
base::Unretained(cookie_monster), net::CookieDeletionInfo(),
Expand Down
2 changes: 1 addition & 1 deletion cobalt/loader/fetch_interceptor_coordinator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ FetchInterceptorCoordinator::FetchInterceptorCoordinator()
void FetchInterceptorCoordinator::TryIntercept(
const GURL& url, bool main_resource,
const net::HttpRequestHeaders& request_headers,
scoped_refptr<base::SingleThreadTaskRunner> callback_task_runner,
scoped_refptr<base::SequencedTaskRunner> callback_task_runner,
base::OnceCallback<void(std::unique_ptr<std::string>)> callback,
base::OnceCallback<void(const net::LoadTimingInfo&)>
report_load_timing_info,
Expand Down
6 changes: 3 additions & 3 deletions cobalt/loader/fetch_interceptor_coordinator.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#include <string>

#include "base/bind.h"
#include "base/single_thread_task_runner.h"
#include "base/sequenced_task_runner.h"
#include "net/base/load_timing_info.h"
#include "net/http/http_request_headers.h"
#include "url/gurl.h"
Expand All @@ -37,7 +37,7 @@ class FetchInterceptor {
virtual void StartFetch(
const GURL& url, bool main_resource,
const net::HttpRequestHeaders& request_headers,
scoped_refptr<base::SingleThreadTaskRunner> callback_task_runner,
scoped_refptr<base::SequencedTaskRunner> callback_task_runner,
base::OnceCallback<void(std::unique_ptr<std::string>)> callback,
base::OnceCallback<void(const net::LoadTimingInfo&)>
report_load_timing_info,
Expand All @@ -58,7 +58,7 @@ class FetchInterceptorCoordinator {
void TryIntercept(
const GURL& url, bool main_resource,
const net::HttpRequestHeaders& request_headers,
scoped_refptr<base::SingleThreadTaskRunner> callback_task_runner,
scoped_refptr<base::SequencedTaskRunner> callback_task_runner,
base::OnceCallback<void(std::unique_ptr<std::string>)> callback,
base::OnceCallback<void(const net::LoadTimingInfo&)>
report_load_timing_info,
Expand Down
2 changes: 1 addition & 1 deletion cobalt/network/network_module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ void NetworkModule::Initialize(const std::string& user_agent_string,
}

void NetworkModule::OnCreate(base::WaitableEvent* creation_event) {
DCHECK(task_runner()->BelongsToCurrentThread());
DCHECK(task_runner()->RunsTasksInCurrentSequence());

net::NetLog* net_log = NULL;
#if defined(ENABLE_NETWORK_LOGGING)
Expand Down
4 changes: 2 additions & 2 deletions cobalt/network/network_module.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#include <string>

#include "base/message_loop/message_loop.h"
#include "base/single_thread_task_runner.h"
#include "base/sequenced_task_runner.h"
#include "base/threading/thread.h"
#include "cobalt/base/event_dispatcher.h"
#include "cobalt/network/cobalt_net_log.h"
Expand Down Expand Up @@ -96,7 +96,7 @@ class NetworkModule {
scoped_refptr<URLRequestContextGetter> url_request_context_getter() const {
return url_request_context_getter_;
}
scoped_refptr<base::SingleThreadTaskRunner> task_runner() const {
scoped_refptr<base::SequencedTaskRunner> task_runner() const {
return thread_->task_runner();
}
storage::StorageManager* storage_manager() const { return storage_manager_; }
Expand Down
7 changes: 4 additions & 3 deletions cobalt/network/persistent_cookie_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "cobalt/network/persistent_cookie_store.h"

#include <memory>
#include <utility>
#include <vector>

#include "base/bind.h"
Expand All @@ -30,7 +31,7 @@ const base::TimeDelta kMaxCookieLifetime = base::TimeDelta::FromDays(365 * 2);

void CookieStorageInit(
const PersistentCookieStore::LoadedCallback& loaded_callback,
scoped_refptr<base::SingleThreadTaskRunner> loaded_callback_task_runner,
scoped_refptr<base::SequencedTaskRunner> loaded_callback_task_runner,
const storage::MemoryStore& memory_store) {
TRACE_EVENT0("cobalt::network", "PersistentCookieStore::CookieStorageInit()");

Expand Down Expand Up @@ -79,7 +80,7 @@ void CookieStorageDeleteCookie(const net::CanonicalCookie& cc,

void SendEmptyCookieList(
const PersistentCookieStore::LoadedCallback& loaded_callback,
scoped_refptr<base::SingleThreadTaskRunner> loaded_callback_task_runner,
scoped_refptr<base::SequencedTaskRunner> loaded_callback_task_runner,
const storage::MemoryStore& memory_store) {
loaded_callback_task_runner->PostTask(
FROM_HERE,
Expand All @@ -96,7 +97,7 @@ void SendEmptyCookieList(

PersistentCookieStore::PersistentCookieStore(
storage::StorageManager* storage,
scoped_refptr<base::SingleThreadTaskRunner> network_task_runner)
scoped_refptr<base::SequencedTaskRunner> network_task_runner)
: storage_(storage), loaded_callback_task_runner_(network_task_runner) {}

PersistentCookieStore::~PersistentCookieStore() {}
Expand Down
6 changes: 3 additions & 3 deletions cobalt/network/persistent_cookie_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#include <string>
#include <vector>

#include "base/single_thread_task_runner.h"
#include "base/sequenced_task_runner.h"
#include "cobalt/storage/storage_manager.h"
#include "net/cookies/cookie_monster.h"

Expand All @@ -29,7 +29,7 @@ class PersistentCookieStore : public net::CookieMonster::PersistentCookieStore {
public:
explicit PersistentCookieStore(
storage::StorageManager* storage,
scoped_refptr<base::SingleThreadTaskRunner> network_task_runner);
scoped_refptr<base::SequencedTaskRunner> network_task_runner);
~PersistentCookieStore() override;

// net::CookieMonster::PersistentCookieStore methods
Expand All @@ -52,7 +52,7 @@ class PersistentCookieStore : public net::CookieMonster::PersistentCookieStore {
storage::StorageManager* storage_;
// This is required because for example cookie store callbacks can only be
// executed on the network thread.
scoped_refptr<base::SingleThreadTaskRunner> loaded_callback_task_runner_;
scoped_refptr<base::SequencedTaskRunner> loaded_callback_task_runner_;

DISALLOW_COPY_AND_ASSIGN(PersistentCookieStore);
};
Expand Down
4 changes: 2 additions & 2 deletions cobalt/network/url_request_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ const char kQUICToggleCommandLongHelp[] =
URLRequestContext::URLRequestContext(
storage::StorageManager* storage_manager, const std::string& custom_proxy,
net::NetLog* net_log, bool ignore_certificate_errors,
scoped_refptr<base::SingleThreadTaskRunner> network_task_runner,
scoped_refptr<base::SequencedTaskRunner> network_task_runner,
persistent_storage::PersistentSettings* persistent_settings)
: ALLOW_THIS_IN_INITIALIZER_LIST(storage_(this))
#if defined(ENABLE_DEBUGGER)
Expand Down Expand Up @@ -230,7 +230,7 @@ void URLRequestContext::SetProxy(const std::string& proxy_rules) {
}

void URLRequestContext::SetEnableQuic(bool enable_quic) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
storage_.http_network_session()->SetEnableQuic(enable_quic);
}

Expand Down
6 changes: 3 additions & 3 deletions cobalt/network/url_request_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

#include "base/basictypes.h"
#include "base/macros.h"
#include "base/threading/thread_checker.h"
#include "base/sequence_checker.h"
#include "cobalt/persistent_storage/persistent_settings.h"
#include "net/cookies/cookie_monster.h"
#include "net/log/net_log.h"
Expand All @@ -43,7 +43,7 @@ class URLRequestContext : public net::URLRequestContext {
URLRequestContext(
storage::StorageManager* storage_manager, const std::string& custom_proxy,
net::NetLog* net_log, bool ignore_certificate_errors,
scoped_refptr<base::SingleThreadTaskRunner> network_task_runner,
scoped_refptr<base::SequencedTaskRunner> network_task_runner,
persistent_storage::PersistentSettings* persistent_settings);
~URLRequestContext() override;

Expand All @@ -54,7 +54,7 @@ class URLRequestContext : public net::URLRequestContext {
bool using_http_cache();

private:
THREAD_CHECKER(thread_checker_);
SEQUENCE_CHECKER(sequence_checker_);
net::URLRequestContextStorage storage_;
scoped_refptr<net::CookieMonster::PersistentCookieStore>
persistent_cookie_store_;
Expand Down
2 changes: 1 addition & 1 deletion cobalt/network/url_request_context_getter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ net::URLRequestContext* URLRequestContextGetter::GetURLRequestContext() {
return url_request_context_;
}

scoped_refptr<base::SingleThreadTaskRunner>
scoped_refptr<base::SequencedTaskRunner>
URLRequestContextGetter::GetNetworkTaskRunner() const {
return network_task_runner_;
}
Expand Down
4 changes: 2 additions & 2 deletions cobalt/network/url_request_context_getter.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@ class URLRequestContextGetter : public net::URLRequestContextGetter {

// Implementation for net::UrlRequestContextGetter.
net::URLRequestContext* GetURLRequestContext() override;
scoped_refptr<base::SingleThreadTaskRunner> GetNetworkTaskRunner()
scoped_refptr<base::SequencedTaskRunner> GetNetworkTaskRunner()
const override;

protected:
virtual ~URLRequestContextGetter();

private:
URLRequestContext* url_request_context_;
scoped_refptr<base::SingleThreadTaskRunner> network_task_runner_;
scoped_refptr<base::SequencedTaskRunner> network_task_runner_;
DISALLOW_COPY_AND_ASSIGN(URLRequestContextGetter);
};

Expand Down

0 comments on commit a16f613

Please sign in to comment.