Skip to content

Commit a056595

Browse files
committedFeb 8, 2024
Address review comments.
1 parent 19e9ec6 commit a056595

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;
@@ -132,7 +133,7 @@ - (BOOL)setValueInternal:(NSDictionary<NSString *, id> *)value logIfNotAssociate
132133

133134
// We serialized properly, so should be good to go on the value. Lock
134135
// around our ivar accesses.
135-
MTRAutoUnfairLock lock(_lock);
136+
std::lock_guard lock(_lock);
136137

137138
_value = [value copy];
138139

@@ -161,13 +162,13 @@ - (BOOL)setValueInternal:(NSDictionary<NSString *, id> *)value logIfNotAssociate
161162

162163
- (NSDictionary<NSString *, id> *)value
163164
{
164-
MTRAutoUnfairLock lock(_lock);
165+
std::lock_guard lock(_lock);
165166
return [_value copy];
166167
}
167168

168169
- (BOOL)associateWithController:(nullable MTRDeviceController *)controller
169170
{
170-
MTRAutoUnfairLock lock(_lock);
171+
std::lock_guard lock(_lock);
171172

172173
MTRDeviceController * existingController = _deviceController;
173174
if (existingController != nil) {
@@ -187,14 +188,14 @@ - (BOOL)associateWithController:(nullable MTRDeviceController *)controller
187188

188189
- (void)invalidate
189190
{
190-
MTRAutoUnfairLock lock(_lock);
191+
std::lock_guard lock(_lock);
191192

192193
_deviceController = nil;
193194
}
194195

195196
- (BOOL)addToCluster:(const app::ConcreteClusterPath &)cluster
196197
{
197-
MTRAutoUnfairLock lock(_lock);
198+
std::lock_guard lock(_lock);
198199

199200
if (_parentCluster.mClusterId != kInvalidClusterId) {
200201
MTR_LOG_ERROR("Cannot add attribute to cluster " ChipLogFormatMEI "; already added to cluster " ChipLogFormatMEI, ChipLogValueMEI(cluster.mClusterId), ChipLogValueMEI(_parentCluster.mClusterId));
@@ -207,13 +208,13 @@ - (BOOL)addToCluster:(const app::ConcreteClusterPath &)cluster
207208

208209
- (void)updateParentCluster:(const app::ConcreteClusterPath &)cluster
209210
{
210-
MTRAutoUnfairLock lock(_lock);
211+
std::lock_guard lock(_lock);
211212
_parentCluster = cluster;
212213
}
213214

214215
- (const chip::app::ConcreteClusterPath &)parentCluster
215216
{
216-
MTRAutoUnfairLock lock(_lock);
217+
std::lock_guard lock(_lock);
217218
return _parentCluster;
218219
}
219220

0 commit comments

Comments
 (0)
Please sign in to comment.