Memory trasher when closing down MYNetwork

[ghmrs356]

I get a 100% repeatable crash with malloc guard turned on, I believe I know what happens but I really really would appreciate if Jens could take a look at this an suggest a fix. This happens in a release app and it's urgent.

First my network startup and shutdown code, so you know the sequence of things:

- (void)startup
{
    if (browser != nil)
        return;

    browser = [[MYBonjourBrowser alloc] initWithServiceType:@"sometype"];
    [browser setUsePrivateConnection:YES];

    registration = [[browser myRegistration] retain];

    listener = [[BLIPListener alloc] initWithPort:12345];
    [listener setPickAvailablePort:YES];

    [browser start];
    [listener setDelegate:self];
    [listener open];

    [registration setPort:[listener port]];
    [registration start];
}

- (void)shutdown
{
    [browser removeObserver:self forKeyPath:@"services"];

    [self setAvailablePeers:nil];

    [connection close];
    [connection release], connection = nil;

    [listener close];
    [listener setDelegate:nil];
    [listener release], listener = nil;

    [registration stop];
    [registration release], registration = nil;

    [browser stop];
    [browser release], browser = nil;
}

In -shutdown the crash happens in the call [registration stop];. Which triggers [MYDNSService cancel] where DNSServiceRefDeallocate(_serviceRef) is called. Then setobj(&connection, nil) is called which in turn triggers [MYDNSConnection dealloc] and [MYDNSConnect close]_ where eventually this is called:

DNSServiceRefDeallocate(_connectionRef);

Since _connectionRef and _serviceRef are one and the same, the second deallocation messes things up good.

I really need a fix for this and I would appreciate suggestions.

[snej]

You're right, MYDNSConnection doesn't own its connection ref unless it's the shared singleton. I think you can fix it by changing -[MYDNSConnection close] so it only calls DNSServiceRefDeallocate if self == sSharedConnection. (That's not the most elegant fix, but it's the one that involves the fewest changes.)

Let me know if that works, and I can update the repo with a cleaner fix that does the same thing.

[ghmrs356]

Actually I went ahead and made a change to [MYDNSService cancel]:

- (void) cancel
{
    if( _serviceRef ) {
        LogTo(DNS,@"Stopped %@",self);

        if ([_connection connectionRef] != _serviceRef) {  /// <<< new check
            DNSServiceRefDeallocate(_serviceRef);
        }
        _serviceRef = NULL;

        setObj(&_connection,nil);
    }
}

The reason is that the documentation on DNSServiceRefDeallocate() specifically says that runloop and socket cleanup must take place prior to deallocating the serviceRef. Since -cancel is called first and the cleanup happens in MYDNSConnection when it's deallocated, this appears to be the sensible solution. Works for me at least.

[snej]

I fixed it your way. Thanks!