Skip to content

Commit c6baaaf

Browse files
authored
Replace acquire+release thread annotation with excludes (flutter#5944)
The behavior of acquire+release annotation handling has changed in https://reviews.llvm.org/D49355 which breaks the build with the new Clang. However, as has been pointed out, the acquire+release isn't the right way to prevent double locking as the annotations negate each other; the correct way is to use excludes or negative requires. Using excludes annotations also requires using std::lock_guard instead of std::unique_lock because the latter doesn't have the thread annotations due to deferred locking which is not needed in Flutter and so std::lock_guard is a sufficient alternative.
1 parent 63ede2e commit c6baaaf

File tree

4 files changed

+13
-15
lines changed

4 files changed

+13
-15
lines changed

lib/ui/isolate_name_server/isolate_name_server.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
namespace blink {
88

99
Dart_Port IsolateNameServer::LookupIsolatePortByName(const std::string& name) {
10-
std::unique_lock<std::mutex> lock(mutex_);
10+
std::lock_guard<std::mutex> lock(mutex_);
1111
return LookupIsolatePortByNameUnprotected(name);
1212
}
1313

@@ -22,7 +22,7 @@ Dart_Port IsolateNameServer::LookupIsolatePortByNameUnprotected(
2222

2323
bool IsolateNameServer::RegisterIsolatePortWithName(Dart_Port port,
2424
const std::string& name) {
25-
std::unique_lock<std::mutex> lock(mutex_);
25+
std::lock_guard<std::mutex> lock(mutex_);
2626
if (LookupIsolatePortByNameUnprotected(name) != ILLEGAL_PORT) {
2727
// Name is already registered.
2828
return false;
@@ -32,7 +32,7 @@ bool IsolateNameServer::RegisterIsolatePortWithName(Dart_Port port,
3232
}
3333

3434
bool IsolateNameServer::RemoveIsolateNameMapping(const std::string& name) {
35-
std::unique_lock<std::mutex> lock(mutex_);
35+
std::lock_guard<std::mutex> lock(mutex_);
3636
auto port_iterator = port_mapping_.find(name);
3737
if (port_iterator == port_mapping_.end()) {
3838
return false;

lib/ui/isolate_name_server/isolate_name_server.h

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313
#include "flutter/fml/synchronization/thread_annotations.h"
1414
#include "third_party/dart/runtime/include/dart_api.h"
1515

16-
#define LOCK_UNLOCK(m) FML_ACQUIRE(m) FML_RELEASE(m)
17-
1816
namespace blink {
1917

2018
class IsolateNameServer {
@@ -24,16 +22,17 @@ class IsolateNameServer {
2422
// Looks up the Dart_Port associated with a given name. Returns ILLEGAL_PORT
2523
// if the name does not exist.
2624
Dart_Port LookupIsolatePortByName(const std::string& name)
27-
LOCK_UNLOCK(mutex_);
25+
FML_LOCKS_EXCLUDED(mutex_);
2826

2927
// Registers a Dart_Port with a given name. Returns true if registration is
3028
// successful, false if the name entry already exists.
3129
bool RegisterIsolatePortWithName(Dart_Port port, const std::string& name)
32-
LOCK_UNLOCK(mutex_);
30+
FML_LOCKS_EXCLUDED(mutex_);
3331

3432
// Removes a name to Dart_Port mapping given a name. Returns true if the
3533
// mapping was successfully removed, false if the mapping does not exist.
36-
bool RemoveIsolateNameMapping(const std::string& name) LOCK_UNLOCK(mutex_);
34+
bool RemoveIsolateNameMapping(const std::string& name)
35+
FML_LOCKS_EXCLUDED(mutex_);
3736

3837
private:
3938
Dart_Port LookupIsolatePortByNameUnprotected(const std::string& name)

lib/ui/plugins/callback_cache.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ std::mutex DartCallbackCache::mutex_;
1414
std::map<int64_t, DartCallbackRepresentation> DartCallbackCache::cache_;
1515

1616
Dart_Handle DartCallbackCache::GetCallback(int64_t handle) {
17-
std::unique_lock<std::mutex> lock(mutex_);
17+
std::lock_guard<std::mutex> lock(mutex_);
1818
auto iterator = cache_.find(handle);
1919
if (iterator != cache_.end()) {
2020
DartCallbackRepresentation cb = iterator->second;
@@ -26,7 +26,7 @@ Dart_Handle DartCallbackCache::GetCallback(int64_t handle) {
2626
int64_t DartCallbackCache::GetCallbackHandle(const std::string& name,
2727
const std::string& class_name,
2828
const std::string& library_path) {
29-
std::unique_lock<std::mutex> lock(mutex_);
29+
std::lock_guard<std::mutex> lock(mutex_);
3030
std::hash<std::string> hasher;
3131
int64_t hash = hasher(name);
3232
hash += hasher(class_name);
@@ -40,7 +40,7 @@ int64_t DartCallbackCache::GetCallbackHandle(const std::string& name,
4040

4141
std::unique_ptr<DartCallbackRepresentation>
4242
DartCallbackCache::GetCallbackInformation(int64_t handle) {
43-
std::unique_lock<std::mutex> lock(mutex_);
43+
std::lock_guard<std::mutex> lock(mutex_);
4444
auto iterator = cache_.find(handle);
4545
if (iterator != cache_.end()) {
4646
return std::make_unique<DartCallbackRepresentation>(iterator->second);

lib/ui/plugins/callback_cache.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
#include "third_party/dart/runtime/include/dart_api.h"
1616

1717
#define DART_CALLBACK_INVALID_HANDLE -1
18-
#define LOCK_UNLOCK(m) FML_ACQUIRE(m) FML_RELEASE(m)
1918

2019
namespace blink {
2120

@@ -30,12 +29,12 @@ class DartCallbackCache {
3029
static int64_t GetCallbackHandle(const std::string& name,
3130
const std::string& class_name,
3231
const std::string& library_path)
33-
LOCK_UNLOCK(mutex_);
32+
FML_LOCKS_EXCLUDED(mutex_);
3433

35-
static Dart_Handle GetCallback(int64_t handle) LOCK_UNLOCK(mutex_);
34+
static Dart_Handle GetCallback(int64_t handle) FML_LOCKS_EXCLUDED(mutex_);
3635

3736
static std::unique_ptr<DartCallbackRepresentation> GetCallbackInformation(
38-
int64_t handle) LOCK_UNLOCK(mutex_);
37+
int64_t handle) FML_LOCKS_EXCLUDED(mutex_);
3938

4039
private:
4140
static Dart_Handle LookupDartClosure(const std::string& name,

0 commit comments

Comments
 (0)