Skip to content

Commit

Permalink
Revert "Guard the service protocol's global handlers list with a read…
Browse files Browse the repository at this point in the history
…er/writer lock (flutter#6888) flutter#6895" (flutter#6899)
  • Loading branch information
tvolkert authored Nov 18, 2018
1 parent b44b94e commit b6e9375
Show file tree
Hide file tree
Showing 10 changed files with 12 additions and 223 deletions.
6 changes: 0 additions & 6 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,6 @@ FILE: ../../../flutter/fml/platform/posix/file_posix.cc
FILE: ../../../flutter/fml/platform/posix/mapping_posix.cc
FILE: ../../../flutter/fml/platform/posix/native_library_posix.cc
FILE: ../../../flutter/fml/platform/posix/paths_posix.cc
FILE: ../../../flutter/fml/platform/posix/shared_mutex_posix.cc
FILE: ../../../flutter/fml/platform/posix/shared_mutex_posix.h
FILE: ../../../flutter/fml/platform/win/errors_win.cc
FILE: ../../../flutter/fml/platform/win/errors_win.h
FILE: ../../../flutter/fml/platform/win/file_win.cc
Expand All @@ -178,13 +176,9 @@ 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
FILE: ../../../flutter/fml/synchronization/shared_mutex.h
FILE: ../../../flutter/fml/synchronization/shared_mutex_std.cc
FILE: ../../../flutter/fml/synchronization/shared_mutex_std.h
FILE: ../../../flutter/fml/synchronization/thread_annotations.h
FILE: ../../../flutter/fml/synchronization/thread_annotations_unittest.cc
FILE: ../../../flutter/fml/synchronization/thread_checker.h
Expand Down
8 changes: 0 additions & 8 deletions fml/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,8 @@ 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/shared_mutex.h",
"synchronization/thread_annotations.h",
"synchronization/thread_checker.h",
"synchronization/waitable_event.cc",
Expand Down Expand Up @@ -85,12 +83,6 @@ source_set("fml") {

libs = []

if (is_ios || is_mac) {
sources += [ "platform/posix/shared_mutex_posix.cc" ]
} else {
sources += [ "synchronization/shared_mutex_std.cc" ]
}

if (is_ios || is_mac) {
sources += [
"platform/darwin/cf_utils.cc",
Expand Down
30 changes: 0 additions & 30 deletions fml/platform/posix/shared_mutex_posix.cc

This file was deleted.

28 changes: 0 additions & 28 deletions fml/platform/posix/shared_mutex_posix.h

This file was deleted.

36 changes: 0 additions & 36 deletions fml/synchronization/atomic_object.h

This file was deleted.

49 changes: 0 additions & 49 deletions fml/synchronization/shared_mutex.h

This file was deleted.

25 changes: 0 additions & 25 deletions fml/synchronization/shared_mutex_std.cc

This file was deleted.

28 changes: 0 additions & 28 deletions fml/synchronization/shared_mutex_std.h

This file was deleted.

18 changes: 9 additions & 9 deletions runtime/service_protocol.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,30 +45,29 @@ ServiceProtocol::ServiceProtocol()
kRunInViewExtensionName,
kFlushUIThreadTasksExtensionName,
kSetAssetBundlePathExtensionName,
}),
handlers_mutex_(fml::SharedMutex::Create()) {}
}) {}

ServiceProtocol::~ServiceProtocol() {
ToggleHooks(false);
}

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

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

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

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

fml::SharedLock lock(*handlers_mutex_);
std::lock_guard<std::mutex> lock(handlers_mutex_);

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

bool ServiceProtocol::HandleListViewsMethod(
rapidjson::Document& response) const {
fml::SharedLock lock(*handlers_mutex_);
// Collect handler descriptions on their respective task runners.
std::lock_guard<std::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.Load());
handler.second);
}

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

#include <map>
#include <mutex>
#include <set>
#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/shared_mutex.h"
#include "flutter/fml/synchronization/thread_annotations.h"
#include "flutter/fml/task_runner.h"
#include "rapidjson/document.h"
Expand Down Expand Up @@ -73,8 +72,8 @@ class ServiceProtocol {

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

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

0 comments on commit b6e9375

Please sign in to comment.