From 14619f290521d0d9312a26bff26aa226f2d607e2 Mon Sep 17 00:00:00 2001 From: Graham--M Date: Sat, 18 Dec 2021 11:55:21 +0000 Subject: [PATCH] Fix autorelease pool emptying when new references are added (#218) * Add test for emptying autorelease pool * Fix arc autorelease pool emptying when adding further references When releasing a reference in the arc autorelease pool, it is possible and anticipated that new references may added to the pool. This fix addresses an edge case where releasing a reference in the same pool page as the stop position can add more references which cause the insertion of a new page and emptyPool() returns early after emptying the new page. --- Test/CMakeLists.txt | 1 + Test/FastARCPool.m | 41 +++++++++++++++++++++++++++++++++++++++++ arc.mm | 28 ++++++++++++++-------------- 3 files changed, 56 insertions(+), 14 deletions(-) create mode 100644 Test/FastARCPool.m diff --git a/Test/CMakeLists.txt b/Test/CMakeLists.txt index 3cc960f..c9994b4 100644 --- a/Test/CMakeLists.txt +++ b/Test/CMakeLists.txt @@ -27,6 +27,7 @@ set(TESTS Category.m ExceptionTest.m FastARC.m + FastARCPool.m FastRefCount.m Forward.m ManyManySelectors.m diff --git a/Test/FastARCPool.m b/Test/FastARCPool.m new file mode 100644 index 0000000..0debe28 --- /dev/null +++ b/Test/FastARCPool.m @@ -0,0 +1,41 @@ +#include "Test.h" + +#define POOL_SIZE (4096 / sizeof(void*)) + +static BOOL called; + +@interface Canary : Test +@end +@implementation Canary +- (void)dealloc +{ + called = YES; + [super dealloc]; +} +@end + +@interface Creator : Test +@end +@implementation Creator +- (void)dealloc +{ + // Add a new page of autorelease references to see if we can still release + // the reference on the canary object. + for (int i = 0; i < POOL_SIZE; i++) + [[Test new] autorelease]; + [super dealloc]; +} +@end + +int main() +{ + called = NO; + @autoreleasepool + { + [[Canary new] autorelease]; + [[Creator new] autorelease]; + } + assert(called == YES); + + return 0; +} diff --git a/arc.mm b/arc.mm index 0619b2b..f1f2b8b 100644 --- a/arc.mm +++ b/arc.mm @@ -169,28 +169,28 @@ static void emptyPool(struct arc_tls *tls, void *stop) stopPool = stopPool->previous; } } - while (tls->pool != stopPool) - { - while (tls->pool->insert > tls->pool->pool) + do { + while (tls->pool != stopPool) { - tls->pool->insert--; - // This may autorelease some other objects, so we have to work in - // the case where the autorelease pool is extended during a -release. - release(*tls->pool->insert); + while (tls->pool->insert > tls->pool->pool) + { + tls->pool->insert--; + // This may autorelease some other objects, so we have to work in + // the case where the autorelease pool is extended during a -release. + release(*tls->pool->insert); + } + void *old = tls->pool; + tls->pool = tls->pool->previous; + free(old); } - void *old = tls->pool; - tls->pool = tls->pool->previous; - free(old); - } - if (NULL != tls->pool) - { + if (NULL == tls->pool) break; while ((stop == NULL || (tls->pool->insert > stop)) && (tls->pool->insert > tls->pool->pool)) { tls->pool->insert--; release(*tls->pool->insert); } - } + } while (tls->pool != stopPool); //fprintf(stderr, "New insert: %p. Stop: %p\n", tls->pool->insert, stop); }