gnustep / libobjc2

Objective-C runtime library intended for use with Clang.
http://www.gnustep.org/
MIT License
426 stars 116 forks source link

emptyPool can fail to empty pool completely #217

Closed lcampbel closed 2 years ago

lcampbel commented 2 years ago

In arc.mm/emptyPool, in this snippet:

    while (tls->pool != stopPool)
    {
            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);
    }
    if (NULL != tls->pool)
    {
            while ((stop == NULL || (tls->pool->insert > stop)) &&
                   (tls->pool->insert > tls->pool->pool))
            {
                    tls->pool->insert--;
                    release(*tls->pool->insert);
            }
    }

if the second release() calls something (a dealloc method typically) that autoreleases one or more objects, a new arc pool (I'm calling it that to distinguish it from an autorelease pool) could be pushed; in that case, the loop will stop when it's finished emptying the new pool, possibly leaving the previous pool partially unemptied.

I think this can be fixed by wrapping the entire snippet above inside

    while (tls->pool != stopPool)
Graham--M commented 2 years ago

Thanks. I can trigger it with this test case. I'll try make a PR with the fix.

#include "Test.h"

static BOOL called;

@interface Canary : Test
@end
@implementation Canary
- (void)dealloc
{
    called = YES;
    [super dealloc];
}
@end

@interface Creator : Test
@end
@implementation Creator
- (void)dealloc
{
    for (int i = 0; i < 512; i++)
        [[Test new] autorelease];
    [super dealloc];
}
@end

int main()
{
    called = NO;
    @autoreleasepool
    {
        [[Canary new] autorelease];
        [[Creator new] autorelease];
    }
    assert(called == YES);

    return 0;
}