From 0c1a6063f6d80c99e60d13462c89ab44018fb9b6 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 27 Jan 2025 13:20:10 -0500 Subject: [PATCH] Fix thread race in test008_pairingAfterCancellation_DeviceAttestationVerification (#37191) The attestation delegate is called on some random framework-internal queue, so it setting the boolean could race with the test main body reading the boolean, which could cause TSan failures, as well as outright failures in some cases. Adding a sleep(5) into the delegate callback before setting the boolean made the test fail reliably. The fix is to just use an expectation to track the "has the delegate been called?" state, since that will handle synchronization for us. --- src/darwin/Framework/CHIPTests/MTRPairingTests.m | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/darwin/Framework/CHIPTests/MTRPairingTests.m b/src/darwin/Framework/CHIPTests/MTRPairingTests.m index e0bac5f2a74ac7..4aade77a11db4f 100644 --- a/src/darwin/Framework/CHIPTests/MTRPairingTests.m +++ b/src/darwin/Framework/CHIPTests/MTRPairingTests.m @@ -480,14 +480,15 @@ - (void)test007_pairingAfterCancellation_FindOperational - (void)test008_pairingAfterCancellation_DeviceAttestationVerification { // Cancel pairing while we are waiting for our client to decide what to do - // with the attestation information we got. - __block BOOL delegateCalled = NO; - __auto_type * attestationDelegate = [[NoOpAttestationDelegate alloc] initWithCallback:^{ - delegateCalled = YES; - } blockCommissioning:YES]; + // with the attestation information we got. Note that the delegate is + // called on some arbitrary queue, so we need to make sure we wait for it to + // actually be called; we can't just have it set a boolean that we then + // check, because that can race. + XCTestExpectation * expectation = [self expectationWithDescription:@"Attestation delegate called"]; + __auto_type * attestationDelegate = [[NoOpAttestationDelegate alloc] initWithExpectation:expectation blockCommissioning:YES]; [self doPairingTestAfterCancellationAtProgress:@"Successfully extended fail-safe timer to handle DA failure" attestationDelegate:attestationDelegate]; - XCTAssertTrue(delegateCalled); + [self waitForExpectations:@[ expectation ] timeout:kPairingTimeoutInSeconds]; } - (void)test009_PairWithReadingEndpointInformation