Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for eDO to manage extra release (from ARC) for non vanilla selectors. #252

Merged
merged 1 commit into from
Jun 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 27 additions & 4 deletions Service/Sources/EDOInvocationMessage.m
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,24 @@ static EDOMethodFamily MethodTypeOfRetainsReturn(const char *methodName, Class t
}
}

/**
* Gets the length of the prefix of the given family type.
*
*
* @param FamilyType The EDOMethodFamily (enum).
* @return The length of the prefix associated with the EDOMethodFamily.
*/
static size_t PrefixLengthForEDOMethodFamily(EDOMethodFamily FamilyType) {
int familySize = sizeof(kRetainReturnsMethodsFamily) / sizeof(MethodFamily);
for (int methodIdx = 0; methodIdx < familySize; methodIdx++) {
MethodFamily methodFam = kRetainReturnsMethodsFamily[methodIdx];
if (methodFam.family == FamilyType) {
return methodFam.length;
}
}
return 0;
}

static EDORemoteException *CreateRemoteException(id localException) {
if (!localException) {
return nil;
Expand Down Expand Up @@ -198,8 +216,11 @@ - (instancetype)initWithReturnValue:(EDOBoxedValueType *)value
_returnValue = value;
_exception = exception;
_outValues = outValues;
_returnRetained = MethodTypeOfRetainsReturn(request.selectorName.UTF8String, targetClass) !=
EDOMethodFamilyNone;

EDOMethodFamily familyType =
MethodTypeOfRetainsReturn(request.selectorName.UTF8String, targetClass);
_returnRetained = familyType != EDOMethodFamilyNone &&
[request.selectorName length] > PrefixLengthForEDOMethodFamily(familyType);
}
return self;
}
Expand Down Expand Up @@ -429,7 +450,9 @@ + (EDORequestHandler)requestHandler {
returnValue = request.returnByValue ? [EDOParameter parameterWithObject:obj]
: BOX_VALUE(obj, nil, service, hostPort);
}
if (family != EDOMethodFamilyNone) {

if (family != EDOMethodFamilyNone &&
[request.selectorName length] > PrefixLengthForEDOMethodFamily(family)) {
// We need to do an extra release here because the method return is not autoreleased,
// and because the invocation is dynamically created, ARC won't insert an extra release
// for us.
Expand Down Expand Up @@ -526,4 +549,4 @@ - (NSString *)description {
self.messageID, self.target, self.selectorName];
}

@end
@end
83 changes: 83 additions & 0 deletions Service/Tests/FunctionalTests/EDOServiceUIMemoryTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,48 @@ - (void)testStubClassAllocEqualsInit {
XCTAssertEqual(testDummy.value, 10);
}

/** Verifies the retain count of the vanilla `new` method is balanced. */
- (void)testStubClassNewBalancesRetainCount {
__weak EDOTestClassDummy *testDummy;
@autoreleasepool {
// The autoreleasepool to assure the inserted releases within the scope and all the temporary
// objects if any will be reclaimed.
id strongRef = [EDOTestClassDummy new]; // NOLINT
testDummy = strongRef;
XCTAssertNotNil(testDummy);
}
// Ensure the appropriate value is retrieved from the object.
XCTAssertNil(testDummy);
}

- (void)testStubClassCopyBalancesRetainCount {
EDOTestClassDummy *testDummy;
@autoreleasepool {
// The autoreleasepool to assure the inserted releases within the scope and all the temporary
// objects if any will be reclaimed.
// +alloc should return the same instance as from -init.
EDOTestClassDummy *strongRef = [EDOTestClassDummy alloc];
testDummy = [strongRef copy];
XCTAssertNotNil(testDummy);
}
// Ensure the appropriate value is retrieved from the object.
XCTAssertNotNil(testDummy);
}

- (void)testStubClassMutableCopyBalancesRetainCount {
EDOTestClassDummy *testDummy;
@autoreleasepool {
// The autoreleasepool to assure the inserted releases within the scope and all the temporary
// objects if any will be reclaimed.
// +alloc should return the same instance as from -init.
EDOTestClassDummy *strongRef = [EDOTestClassDummy alloc];
testDummy = [strongRef mutableCopy];
XCTAssertNotNil(testDummy);
}
// Ensure the appropriate value is retrieved from the object.
XCTAssertNotNil(testDummy);
}

- (void)testAllocFamilyRetainsReturn {
EDOTestClassDummy *testDummy1, *testDummy2, *testDummy3;
@autoreleasepool {
Expand All @@ -76,6 +118,47 @@ - (void)testAllocFamilyRetainsReturn {
XCTAssertEqual(testDummy3.value, 30);
}

- (void)testNewPrefixedFamilyRetainsReturn {
__weak EDOTestClassDummy *testDummy;
@autoreleasepool {
// The autoreleasepool to assure the inserted releases within the scope and all the temporary
// objects if any will be reclaimed.
id strongRef = [EDOTestClassDummy newBuilder];
testDummy = strongRef;
XCTAssertNotNil(testDummy);
}
// Ensure the appropriate value is retrieved from the object.
XCTAssertNil(testDummy);
}

- (void)testCopyPrefixedFamilyRetainsReturn {
EDOTestClassDummy *testDummy;
@autoreleasepool {
// The autoreleasepool to assure the inserted releases within the scope and all the temporary
// objects if any will be reclaimed.
// +alloc should return the same instance as from -init.
EDOTestClassDummy *strongRef = [EDOTestClassDummy alloc];
testDummy = [strongRef copyDummy];
XCTAssertNotNil(testDummy);
}
// Ensure the appropriate value is retrieved from the object.
XCTAssertNotNil(testDummy);
}

- (void)testMutableCopyPrefixedFamilyRetainsReturn {
EDOTestClassDummy *testDummy;
@autoreleasepool {
// The autoreleasepool to assure the inserted releases within the scope and all the temporary
// objects if any will be reclaimed.
// +alloc should return the same instance as from -init.
EDOTestClassDummy *strongRef = [EDOTestClassDummy alloc];
testDummy = [strongRef mutableCopyDummy];
XCTAssertNotNil(testDummy);
}
// Ensure the appropriate value is retrieved from the object.
XCTAssertNotNil(testDummy);
}

- (void)testAllocRemoteValueType {
XCTAssertThrowsSpecificNamed([[EDO_REMOTE_CLASS(NSData, EDOTEST_APP_SERVICE_PORT) alloc] init],
EDORemoteException, EDOServiceAllocValueTypeException);
Expand Down
30 changes: 29 additions & 1 deletion Service/Tests/TestsBundle/EDOTestClassDummy.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,35 @@
*/
+ (instancetype)allocateDummy;

/**
* A method with the new prefix belonging to the new family.
* http://clang.llvm.org/docs/AutomaticReferenceCounting.html#method-families
*/
+ (instancetype)newBuilder;

/**
* This is the method invoked behind the scenes for vanilla copy.
*/
- (instancetype)copyWithZone:(NSZone*)zone;

/**
* A method with the copy prefix belonging to the copy family.
* http://clang.llvm.org/docs/AutomaticReferenceCounting.html#method-families
*/
- (instancetype)copyDummy;

/**
* This is the method invoked behind the scenes for vanilla mutableCopy.
*/
- (instancetype)mutableCopyWithZone:(NSZone*)zone;

/**
* A method with the mutableCopy prefix belonging to the mutableCopy family.
* http://clang.llvm.org/docs/AutomaticReferenceCounting.html#method-families
*/
- (instancetype)mutableCopyDummy;

+ (int)classMethodWithInt:(int)value;
+ (EDOTestClassDummy *)classMethodWithIdReturn:(int)value;
+ (EDOTestClassDummy*)classMethodWithIdReturn:(int)value;

@end
28 changes: 28 additions & 0 deletions Service/Tests/TestsBundle/EDOTestClassDummy.m
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,34 @@ + (instancetype)allocateDummy {
return [self alloc];
}

+ (instancetype)newBuilder {
return [self new];
}

- (instancetype)copyWithZone:(NSZone *)zone {
EDOTestClassDummy *newInstance = [[EDOTestClassDummy alloc] init];
newInstance->_value = _value;
return newInstance;
}

- (instancetype)copyDummy {
EDOTestClassDummy *newInstance = [[EDOTestClassDummy alloc] init];
newInstance->_value = _value;
return newInstance;
}

- (instancetype)mutableCopyWithZone:(NSZone *)zone {
EDOTestClassDummy *newInstance = [[EDOTestClassDummy alloc] init];
newInstance->_value = _value;
return newInstance;
}

- (instancetype)mutableCopyDummy {
EDOTestClassDummy *newInstance = [[EDOTestClassDummy alloc] init];
newInstance->_value = _value;
return newInstance;
}

+ (int)classMethodWithInt:(int)value {
return [self classMethodWithIdReturn:value].value + 9;
}
Expand Down