Skip to content

Commit

Permalink
Change how the Matter framework handles Platform::Memory. (#19273)
Browse files Browse the repository at this point in the history
Since for the malloc case the Platform::Memory is just debugging bits,
and since its init is not particularly threadsafe, just do a
dispatch_once init whenever we might need it, and don't worry about
shutting it down.
  • Loading branch information
bzbarsky-apple authored Jun 8, 2022
1 parent 7b3fb84 commit fbce051
Show file tree
Hide file tree
Showing 9 changed files with 96 additions and 37 deletions.
8 changes: 8 additions & 0 deletions src/darwin/Framework/CHIP.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
513DDB8A2761F6F900DAA01A /* CHIPAttributeTLVValueDecoder.mm in Sources */ = {isa = PBXBuildFile; fileRef = 513DDB892761F6F900DAA01A /* CHIPAttributeTLVValueDecoder.mm */; };
51431AF927D2973E008A7943 /* CHIPIMDispatch.mm in Sources */ = {isa = PBXBuildFile; fileRef = 51431AF827D2973E008A7943 /* CHIPIMDispatch.mm */; };
51431AFB27D29CA4008A7943 /* ota-provider.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 51431AFA27D29CA4008A7943 /* ota-provider.cpp */; };
515C1C6F284F9FFB00A48F0C /* MTRMemory.mm in Sources */ = {isa = PBXBuildFile; fileRef = 515C1C6D284F9FFB00A48F0C /* MTRMemory.mm */; };
515C1C70284F9FFB00A48F0C /* MTRMemory.h in Headers */ = {isa = PBXBuildFile; fileRef = 515C1C6E284F9FFB00A48F0C /* MTRMemory.h */; };
517BF3F0282B62B800A8B7DB /* MTRCertificates.h in Headers */ = {isa = PBXBuildFile; fileRef = 517BF3EE282B62B800A8B7DB /* MTRCertificates.h */; settings = {ATTRIBUTES = (Public, ); }; };
517BF3F1282B62B800A8B7DB /* MTRCertificates.mm in Sources */ = {isa = PBXBuildFile; fileRef = 517BF3EF282B62B800A8B7DB /* MTRCertificates.mm */; };
517BF3F3282B62CB00A8B7DB /* MatterCertificateTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 517BF3F2282B62CB00A8B7DB /* MatterCertificateTests.m */; };
Expand Down Expand Up @@ -152,6 +154,8 @@
513DDB892761F6F900DAA01A /* CHIPAttributeTLVValueDecoder.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = CHIPAttributeTLVValueDecoder.mm; path = "zap-generated/CHIPAttributeTLVValueDecoder.mm"; sourceTree = "<group>"; };
51431AF827D2973E008A7943 /* CHIPIMDispatch.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CHIPIMDispatch.mm; sourceTree = "<group>"; };
51431AFA27D29CA4008A7943 /* ota-provider.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = "ota-provider.cpp"; path = "../../../app/clusters/ota-provider/ota-provider.cpp"; sourceTree = "<group>"; };
515C1C6D284F9FFB00A48F0C /* MTRMemory.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MTRMemory.mm; sourceTree = "<group>"; };
515C1C6E284F9FFB00A48F0C /* MTRMemory.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRMemory.h; sourceTree = "<group>"; };
517BF3EE282B62B800A8B7DB /* MTRCertificates.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRCertificates.h; sourceTree = "<group>"; };
517BF3EF282B62B800A8B7DB /* MTRCertificates.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MTRCertificates.mm; sourceTree = "<group>"; };
517BF3F2282B62CB00A8B7DB /* MatterCertificateTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MatterCertificateTests.m; sourceTree = "<group>"; };
Expand Down Expand Up @@ -355,6 +359,8 @@
5A7947E227C0101200434CF2 /* CHIPDeviceController+XPC.h */,
517BF3EE282B62B800A8B7DB /* MTRCertificates.h */,
517BF3EF282B62B800A8B7DB /* MTRCertificates.mm */,
515C1C6E284F9FFB00A48F0C /* MTRMemory.h */,
515C1C6D284F9FFB00A48F0C /* MTRMemory.mm */,
5A7947E327C0129500434CF2 /* CHIPDeviceController+XPC.m */,
B20252912459E34F00F97062 /* Info.plist */,
998F286C26D55E10001846C6 /* CHIPKeypair.h */,
Expand Down Expand Up @@ -419,6 +425,7 @@
99D466E12798936D0089A18F /* CHIPCommissioningParameters.h in Headers */,
5136661528067D550025EDAE /* MatterControllerFactory_Internal.h in Headers */,
75C645A42825AAC3007E2C29 /* MatterClusterConstants.h in Headers */,
515C1C70284F9FFB00A48F0C /* MTRMemory.h in Headers */,
B289D4212639C0D300D4E314 /* CHIPOnboardingPayloadParser.h in Headers */,
513DDB862761F69300DAA01A /* CHIPAttributeTLVValueDecoder_Internal.h in Headers */,
2CB7163F252F731E0026E2BB /* CHIPDevicePairingDelegate.h in Headers */,
Expand Down Expand Up @@ -580,6 +587,7 @@
99AECC802798A57F00B6355B /* CHIPCommissioningParameters.m in Sources */,
2CB7163C252E8A7C0026E2BB /* CHIPDevicePairingDelegateBridge.mm in Sources */,
997DED162695343400975E97 /* CHIPThreadOperationalDataset.mm in Sources */,
515C1C6F284F9FFB00A48F0C /* MTRMemory.mm in Sources */,
27A53C1827FBC6920053F131 /* CHIPAttestationTrustStoreBridge.mm in Sources */,
998F287126D56940001846C6 /* CHIPP256KeypairBridge.mm in Sources */,
5136661428067D550025EDAE /* MatterControllerFactory.mm in Sources */,
Expand Down
2 changes: 2 additions & 0 deletions src/darwin/Framework/CHIP/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ static_library("framework") {
"CHIPSetupPayload.mm",
"MTRCertificates.h",
"MTRCertificates.mm",
"MTRMemory.h",
"MTRMemory.mm",
"MatterControllerFactory.h",
"MatterControllerFactory.mm",
"MatterControllerFactory_Internal.h",
Expand Down
1 change: 0 additions & 1 deletion src/darwin/Framework/CHIP/CHIPDeviceController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
#include <credentials/FabricTable.h>
#include <credentials/GroupDataProvider.h>
#include <lib/core/CHIPVendorIdentifiers.hpp>
#include <lib/support/CHIPMem.h>
#include <platform/PlatformManager.h>
#include <setup_payload/ManualSetupPayloadGenerator.h>
#include <system/SystemClock.h>
Expand Down
8 changes: 2 additions & 6 deletions src/darwin/Framework/CHIP/CHIPManualSetupPayloadParser.mm
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
#import "CHIPError_Internal.h"
#import "CHIPLogging.h"
#import "CHIPSetupPayload_Internal.h"
#import "MTRMemory.h"

#import <lib/support/CHIPMem.h>
#import <setup_payload/ManualSetupPayloadParser.h>
#import <setup_payload/SetupPayload.h>

Expand All @@ -32,10 +32,7 @@ @implementation CHIPManualSetupPayloadParser {
- (id)initWithDecimalStringRepresentation:(NSString *)decimalStringRepresentation
{
if (self = [super init]) {
if (CHIP_NO_ERROR != chip::Platform::MemoryInit()) {
CHIP_LOG_ERROR("Error: couldn't initialize platform memory");
return self;
}
[MTRMemory ensureInit];
_decimalStringRepresentation = decimalStringRepresentation;
_chipManualSetupPayloadParser = new chip::ManualSetupPayloadParser(std::string([decimalStringRepresentation UTF8String]));
}
Expand Down Expand Up @@ -69,7 +66,6 @@ - (void)dealloc
{
delete _chipManualSetupPayloadParser;
_chipManualSetupPayloadParser = nullptr;
chip::Platform::MemoryShutdown();
}

@end
8 changes: 2 additions & 6 deletions src/darwin/Framework/CHIP/CHIPQRCodeSetupPayloadParser.mm
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
#import "CHIPError_Internal.h"
#import "CHIPLogging.h"
#import "CHIPSetupPayload_Internal.h"
#import "MTRMemory.h"

#import <lib/support/CHIPMem.h>
#import <setup_payload/QRCodeSetupPayloadParser.h>
#import <setup_payload/SetupPayload.h>

Expand All @@ -32,10 +32,7 @@ @implementation CHIPQRCodeSetupPayloadParser {
- (id)initWithBase38Representation:(NSString *)base38Representation
{
if (self = [super init]) {
if (CHIP_NO_ERROR != chip::Platform::MemoryInit()) {
CHIP_LOG_ERROR("Error: couldn't initialize platform memory");
return self;
}
[MTRMemory ensureInit];
_base38Representation = base38Representation;
_chipQRCodeSetupPayloadParser = new chip::QRCodeSetupPayloadParser(std::string([base38Representation UTF8String]));
}
Expand Down Expand Up @@ -69,7 +66,6 @@ - (void)dealloc
{
delete _chipQRCodeSetupPayloadParser;
_chipQRCodeSetupPayloadParser = nullptr;
chip::Platform::MemoryShutdown();
}

@end
23 changes: 7 additions & 16 deletions src/darwin/Framework/CHIP/MTRCertificates.mm
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,16 @@
#import "CHIPError_Internal.h"
#import "CHIPOperationalCredentialsDelegate.h"
#import "CHIPP256KeypairBridge.h"
#import "MTRMemory.h"
#import "NSDataSpanConversion.h"

#include <credentials/CHIPCert.h>
#include <crypto/CHIPCryptoPAL.h>
#include <lib/support/CHIPMem.h>

using namespace chip;
using namespace chip::Crypto;
using namespace chip::Credentials;

// RAII helper for doing MemoryInit/MemoryShutdown, just in case the underlying
// Matter APIs we are using use Platform::Memory. MemoryInit/MemoryShutdown are
// refcounted, so it's OK if we use AutoPlatformMemory after MemoryInit has
// already happened elsewhere.
struct AutoPlatformMemory {
AutoPlatformMemory() { Platform::MemoryInit(); }
~AutoPlatformMemory() { Platform::MemoryShutdown(); }
};

@implementation MTRCertificates

+ (nullable NSData *)generateRootCertificate:(id<CHIPKeypair>)keypair
Expand All @@ -46,7 +37,7 @@ + (nullable NSData *)generateRootCertificate:(id<CHIPKeypair>)keypair
{
NSLog(@"Generating root certificate");

AutoPlatformMemory platformMemory;
[MTRMemory ensureInit];

NSData * rootCert = nil;
CHIP_ERROR err = CHIPOperationalCredentialsDelegate::GenerateRootCertificate(keypair, issuerId, fabricId, &rootCert);
Expand All @@ -70,7 +61,7 @@ + (nullable NSData *)generateIntermediateCertificate:(id<CHIPKeypair>)rootKeypai
{
NSLog(@"Generating intermediate certificate");

AutoPlatformMemory platformMemory;
[MTRMemory ensureInit];

NSData * intermediate = nil;
CHIP_ERROR err = CHIPOperationalCredentialsDelegate::GenerateIntermediateCertificate(
Expand All @@ -96,7 +87,7 @@ + (nullable NSData *)generateOperationalCertificate:(id<CHIPKeypair>)signingKeyp
{
NSLog(@"Generating operational certificate");

AutoPlatformMemory platformMemory;
[MTRMemory ensureInit];

NSData * opcert = nil;
CHIP_ERROR err = CHIPOperationalCredentialsDelegate::GenerateOperationalCertificate(
Expand All @@ -114,7 +105,7 @@ + (nullable NSData *)generateOperationalCertificate:(id<CHIPKeypair>)signingKeyp

+ (BOOL)keypair:(id<CHIPKeypair>)keypair matchesCertificate:(NSData *)certificate
{
AutoPlatformMemory platformMemory;
[MTRMemory ensureInit];

P256PublicKey keypairPubKey;
CHIP_ERROR err = CHIPP256KeypairBridge::MatterPubKeyFromSecKeyRef(keypair.pubkey, &keypairPubKey);
Expand All @@ -137,7 +128,7 @@ + (BOOL)keypair:(id<CHIPKeypair>)keypair matchesCertificate:(NSData *)certificat

+ (BOOL)isCertificate:(NSData *)certificate1 equalTo:(NSData *)certificate2
{
AutoPlatformMemory platformMemory;
[MTRMemory ensureInit];

P256PublicKey pubKey1;
CHIP_ERROR err = ExtractPubkeyFromX509Cert(AsByteSpan(certificate1), pubKey1);
Expand Down Expand Up @@ -179,7 +170,7 @@ + (BOOL)isCertificate:(NSData *)certificate1 equalTo:(NSData *)certificate2
+ (nullable NSData *)generateCertificateSigningRequest:(id<CHIPKeypair>)keypair
error:(NSError * __autoreleasing _Nullable * _Nullable)error
{
AutoPlatformMemory platformMemory;
[MTRMemory ensureInit];

CHIPP256KeypairBridge keypairBridge;
CHIP_ERROR err = CHIP_NO_ERROR;
Expand Down
40 changes: 40 additions & 0 deletions src/darwin/Framework/CHIP/MTRMemory.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/**
* Copyright (c) 2022 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#pragma once

/**
* Utility to initialize the Matter memory subsystem. Not a public framework
* header.
*/

#import <Foundation/Foundation.h>

NS_ASSUME_NONNULL_BEGIN

@interface MTRMemory : NSObject
/**
* Ensure Matter Platform::Memory is initialized. This only needs to happen
* once per process, because in practice we just use malloc/free so there is
* nothing to initialize, so this just needs to happen to avoid debug
* assertions. This class handles ensuring the initialization only happens
* once.
*/
+ (void)ensureInit;

@end

NS_ASSUME_NONNULL_END
32 changes: 32 additions & 0 deletions src/darwin/Framework/CHIP/MTRMemory.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* Copyright (c) 2022 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#import "MTRMemory.h"

#include <lib/support/CHIPMem.h>

@implementation MTRMemory

+ (void)ensureInit
{
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
// The malloc version of MemoryInit never fails.
chip::Platform::MemoryInit();
});
}

@end
11 changes: 3 additions & 8 deletions src/darwin/Framework/CHIP/MatterControllerFactory.mm
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#import "CHIPP256KeypairBridge.h"
#import "CHIPPersistentStorageDelegateBridge.h"
#import "MTRCertificates.h"
#import "MTRMemory.h"
#import "NSDataSpanConversion.h"

#include <controller/CHIPDeviceControllerFactory.h>
Expand All @@ -41,7 +42,6 @@
using namespace chip;
using namespace chip::Controller;

static NSString * const kErrorMemoryInit = @"Init Memory failure";
static NSString * const kErrorPersistentStorageInit = @"Init failure while creating a persistent storage delegate";
static NSString * const kErrorAttestationTrustStoreInit = @"Init failure while creating the attestation trust store";
static NSString * const kInfoFactoryShutdown = @"Shutting down the Matter controller factory";
Expand Down Expand Up @@ -89,10 +89,7 @@ - (instancetype)init
_isRunning = NO;
_chipWorkQueue = DeviceLayer::PlatformMgrImpl().GetWorkQueue();
_controllerFactory = &DeviceControllerFactory::GetInstance();
CHIP_ERROR errorCode = Platform::MemoryInit();
if ([self checkForInitError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorMemoryInit]) {
return nil;
}
[MTRMemory ensureInit];

_groupStorageDelegate = new chip::TestPersistentStorageDelegate();
if ([self checkForInitError:(_groupStorageDelegate != nullptr) logMsg:kErrorGroupProviderInit]) {
Expand All @@ -106,7 +103,7 @@ - (instancetype)init
}

_groupDataProvider->SetStorageDelegate(_groupStorageDelegate);
errorCode = _groupDataProvider->Init();
CHIP_ERROR errorCode = _groupDataProvider->Init();
if ([self checkForInitError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorGroupProviderInit]) {
return nil;
}
Expand Down Expand Up @@ -152,8 +149,6 @@ - (void)cleanupInitObjects
delete _groupStorageDelegate;
_groupStorageDelegate = nullptr;
}

Platform::MemoryShutdown();
}

- (void)cleanupStartupObjects
Expand Down

0 comments on commit fbce051

Please sign in to comment.