Skip to content

Commit

Permalink
Guard the service protocol's global handlers list with a reader/write…
Browse files Browse the repository at this point in the history
…r lock

The service protocol holds the lock while waiting for completion of service
RPC tasks.  These tasks (specifically hot restart/RunInView) may need to
modify a handler's description data.

Task execution and ServiceProtocol::SetHandlerDescription will obtain a shared
lock to make this possible.  AddHandler and RemoveHandler will obtain an
exclusive lock in order to guard against a handler being deleted while a
service task is running.
  • Loading branch information
jason-simmons committed Nov 17, 2018
1 parent 9296d80 commit 384d895
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 11 deletions.
1 change: 1 addition & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ FILE: ../../../flutter/fml/platform/win/wstring_conversion.h
FILE: ../../../flutter/fml/string_view.cc
FILE: ../../../flutter/fml/string_view.h
FILE: ../../../flutter/fml/string_view_unittest.cc
FILE: ../../../flutter/fml/synchronization/atomic_object.h
FILE: ../../../flutter/fml/synchronization/count_down_latch.cc
FILE: ../../../flutter/fml/synchronization/count_down_latch.h
FILE: ../../../flutter/fml/synchronization/count_down_latch_unittests.cc
Expand Down
1 change: 1 addition & 0 deletions fml/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ source_set("fml") {
"paths.h",
"string_view.cc",
"string_view.h",
"synchronization/atomic_object.h",
"synchronization/count_down_latch.cc",
"synchronization/count_down_latch.h",
"synchronization/thread_annotations.h",
Expand Down
30 changes: 30 additions & 0 deletions fml/synchronization/atomic_object.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include <mutex>

namespace fml {

template <typename T>
class AtomicObject {
public:
AtomicObject() = default;
AtomicObject(T object) : object_(object) {}

T Load() const {
std::lock_guard<std::mutex> lock(mutex_);
return object_;
}

void Store(const T& object) {
std::lock_guard<std::mutex> lock(mutex_);
object_ = object;
}

private:
mutable std::mutex mutex_;
T object_;
};

} // namespace fml
15 changes: 7 additions & 8 deletions runtime/service_protocol.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,21 @@ ServiceProtocol::~ServiceProtocol() {

void ServiceProtocol::AddHandler(Handler* handler,
Handler::Description description) {
std::lock_guard<std::mutex> lock(handlers_mutex_);
std::unique_lock<std::shared_timed_mutex> lock(handlers_mutex_);
handlers_.emplace(handler, description);
}

void ServiceProtocol::RemoveHandler(Handler* handler) {
std::lock_guard<std::mutex> lock(handlers_mutex_);
std::unique_lock<std::shared_timed_mutex> lock(handlers_mutex_);
handlers_.erase(handler);
}

void ServiceProtocol::SetHandlerDescription(Handler* handler,
Handler::Description description) {
std::lock_guard<std::mutex> lock(handlers_mutex_);
std::shared_lock<std::shared_timed_mutex> lock(handlers_mutex_);
auto it = handlers_.find(handler);
if (it != handlers_.end())
it->second = description;
it->second.Store(description);
}

void ServiceProtocol::ToggleHooks(bool set) {
Expand Down Expand Up @@ -175,7 +175,7 @@ bool ServiceProtocol::HandleMessage(fml::StringView method,
return HandleListViewsMethod(response);
}

std::lock_guard<std::mutex> lock(handlers_mutex_);
std::shared_lock<std::shared_timed_mutex> lock(handlers_mutex_);

if (handlers_.size() == 0) {
WriteServerErrorResponse(response,
Expand Down Expand Up @@ -246,12 +246,11 @@ void ServiceProtocol::Handler::Description::Write(

bool ServiceProtocol::HandleListViewsMethod(
rapidjson::Document& response) const {
// Collect handler descriptions on their respective task runners.
std::lock_guard<std::mutex> lock(handlers_mutex_);
std::shared_lock<std::shared_timed_mutex> lock(handlers_mutex_);
std::vector<std::pair<intptr_t, Handler::Description>> descriptions;
for (const auto& handler : handlers_) {
descriptions.emplace_back(reinterpret_cast<intptr_t>(handler.first),
handler.second);
handler.second.Load());
}

auto& allocator = response.GetAllocator();
Expand Down
7 changes: 4 additions & 3 deletions runtime/service_protocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@
#define FLUTTER_RUNTIME_SERVICE_PROTOCOL_H_

#include <map>
#include <mutex>
#include <set>
#include <shared_mutex>
#include <string>

#include "flutter/fml/compiler_specific.h"
#include "flutter/fml/macros.h"
#include "flutter/fml/string_view.h"
#include "flutter/fml/synchronization/atomic_object.h"
#include "flutter/fml/synchronization/thread_annotations.h"
#include "flutter/fml/task_runner.h"
#include "rapidjson/document.h"
Expand Down Expand Up @@ -72,8 +73,8 @@ class ServiceProtocol {

private:
const std::set<fml::StringView> endpoints_;
mutable std::mutex handlers_mutex_;
std::map<Handler*, Handler::Description> handlers_;
mutable std::shared_timed_mutex handlers_mutex_;
std::map<Handler*, fml::AtomicObject<Handler::Description>> handlers_;

FML_WARN_UNUSED_RESULT
static bool HandleMessage(const char* method,
Expand Down

0 comments on commit 384d895

Please sign in to comment.