Skip to content

Commit 7543c19

Browse files
Address review comments.
1 parent 8534a51 commit 7543c19

File tree

2 files changed

+21
-29
lines changed

2 files changed

+21
-29
lines changed

src/darwin/Framework/CHIP/MTRUnfairLock.h

+8-17
Original file line numberDiff line numberDiff line change
@@ -20,27 +20,18 @@
2020

2121
#import <os/lock.h>
2222

23-
class MTRAutoUnfairLock;
23+
#include <mutex>
2424

25-
class MTRUnfairLock
25+
template <>
26+
class std::lock_guard<os_unfair_lock>
2627
{
2728
public:
28-
MTRUnfairLock() { mOSLock = OS_UNFAIR_LOCK_INIT; }
29+
explicit lock_guard(os_unfair_lock & lock) : mLock(lock) { os_unfair_lock_lock(&mLock); }
30+
~lock_guard() { os_unfair_lock_unlock(&mLock); }
2931

30-
void AssertOwner() { os_unfair_lock_assert_owner(&mOSLock); }
32+
lock_guard(const lock_guard &) = delete;
33+
void operator=(const lock_guard &) = delete;
3134

3235
private:
33-
friend class MTRAutoUnfairLock;
34-
os_unfair_lock mOSLock;
35-
};
36-
37-
class MTRAutoUnfairLock
38-
{
39-
public:
40-
MTRAutoUnfairLock(MTRUnfairLock & aLock) : mLock(aLock) { os_unfair_lock_lock(&mLock.mOSLock); }
41-
42-
~MTRAutoUnfairLock() { os_unfair_lock_unlock(&mLock.mOSLock); }
43-
44-
private:
45-
MTRUnfairLock & mLock;
36+
os_unfair_lock & mLock;
4637
};

src/darwin/Framework/CHIP/ServerEndpoint/MTRServerAttribute.mm

+13-12
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,11 @@
3434

3535
MTR_DIRECT_MEMBERS
3636
@implementation MTRServerAttribute {
37-
// _lock always protects access to _deviceController, _value, and _parentCluster.
38-
// _serializedValue is protected when we are modifying it directly while we
39-
// have no _deviceController. Once we have one, _serializedValue is only
40-
// modified on the Matter thread.
41-
MTRUnfairLock _lock;
37+
// _lock always protects access to _deviceController, _value, and
38+
// _parentCluster. _serializedValue is protected when we are modifying it
39+
// directly while we have no _deviceController. Once we have one,
40+
// _serializedValue is only modified on the Matter thread.
41+
os_unfair_lock _lock;
4242
MTRDeviceController * __weak _deviceController;
4343
NSDictionary<NSString *, id> * _value;
4444
app::ConcreteClusterPath _parentCluster;
@@ -75,6 +75,7 @@ - (nullable instancetype)initWithAttributeID:(NSNumber *)attributeID value:(NSDi
7575
return nil;
7676
}
7777

78+
_lock = OS_UNFAIR_LOCK_INIT;
7879
_attributeID = attributeID;
7980
_requiredReadPrivilege = requiredReadPrivilege;
8081
_writable = writable;
@@ -127,7 +128,7 @@ - (BOOL)setValue:(NSDictionary<NSString *, id> *)value
127128

128129
// We serialized properly, so should be good to go on the value. Lock
129130
// around our ivar accesses.
130-
MTRAutoUnfairLock lock(_lock);
131+
std::lock_guard lock(_lock);
131132

132133
_value = [value copy];
133134

@@ -151,13 +152,13 @@ - (BOOL)setValue:(NSDictionary<NSString *, id> *)value
151152

152153
- (NSDictionary<NSString *, id> *)value
153154
{
154-
MTRAutoUnfairLock lock(_lock);
155+
std::lock_guard lock(_lock);
155156
return [_value copy];
156157
}
157158

158159
- (BOOL)associateWithController:(nullable MTRDeviceController *)controller
159160
{
160-
MTRAutoUnfairLock lock(_lock);
161+
std::lock_guard lock(_lock);
161162

162163
MTRDeviceController * existingController = _deviceController;
163164
if (existingController != nil) {
@@ -177,14 +178,14 @@ - (BOOL)associateWithController:(nullable MTRDeviceController *)controller
177178

178179
- (void)invalidate
179180
{
180-
MTRAutoUnfairLock lock(_lock);
181+
std::lock_guard lock(_lock);
181182

182183
_deviceController = nil;
183184
}
184185

185186
- (BOOL)addToCluster:(const app::ConcreteClusterPath &)cluster
186187
{
187-
MTRAutoUnfairLock lock(_lock);
188+
std::lock_guard lock(_lock);
188189

189190
if (_parentCluster.mClusterId != kInvalidClusterId) {
190191
MTR_LOG_ERROR("Cannot add attribute to cluster " ChipLogFormatMEI "; already added to cluster " ChipLogFormatMEI, ChipLogValueMEI(cluster.mClusterId), ChipLogValueMEI(_parentCluster.mClusterId));
@@ -197,13 +198,13 @@ - (BOOL)addToCluster:(const app::ConcreteClusterPath &)cluster
197198

198199
- (void)updateParentCluster:(const app::ConcreteClusterPath &)cluster
199200
{
200-
MTRAutoUnfairLock lock(_lock);
201+
std::lock_guard lock(_lock);
201202
_parentCluster = cluster;
202203
}
203204

204205
- (const chip::app::ConcreteClusterPath &)parentCluster
205206
{
206-
MTRAutoUnfairLock lock(_lock);
207+
std::lock_guard lock(_lock);
207208
return _parentCluster;
208209
}
209210

0 commit comments

Comments
 (0)