From 9e5a70c9ae5164f7e10cca3b4c5f0055c97afe34 Mon Sep 17 00:00:00 2001 From: Christopher Snowhill Date: Sat, 29 Jan 2022 21:10:59 -0800 Subject: [PATCH] Cog Audio: Dealt with a major retain cycle leak This seals up a major memory leak of the playback state whenever a chain is released on stop or on manual track change. CogAudioMulti was retaining the input node due to its listeners, and InputNode was not releasing the listeners when asked to stop running. This is fixed now. Fixes #221 Signed-off-by: Christopher Snowhill --- Audio/AudioPlayer.m | 19 ++++++++++++++++++- Audio/Chain/InputNode.h | 2 ++ Audio/Chain/InputNode.m | 17 +++++++++++++++-- Audio/Chain/OutputNode.m | 1 + Audio/CogPluginMulti.m | 3 ++- Audio/Output/OutputCoreAudio.h | 1 + Audio/Output/OutputCoreAudio.m | 23 ++++++++++++++--------- 7 files changed, 53 insertions(+), 13 deletions(-) diff --git a/Audio/AudioPlayer.m b/Audio/AudioPlayer.m index 06004e2c1..b493b8220 100644 --- a/Audio/AudioPlayer.m +++ b/Audio/AudioPlayer.m @@ -68,7 +68,10 @@ ALog(@"Opening file for playback: %@ at seek offset %f%@", url, time, (paused) ? @", starting paused" : @""); [self waitUntilCallbacksExit]; - output = nil; + if (output) { + [output setShouldContinue:NO]; + output = nil; + } output = [[OutputNode alloc] initWithController:self previous:nil]; [output setup]; [output setVolume: volume]; @@ -135,6 +138,20 @@ //Set shouldoContinue to NO on all things [self setShouldContinue:NO]; [self setPlaybackStatus:CogStatusStopped waitUntilDone:YES]; + + @synchronized(chainQueue) { + for (id anObject in chainQueue) + { + [anObject setShouldContinue:NO]; + } + [chainQueue removeAllObjects]; + endOfInputReached = NO; + if (bufferChain) + { + bufferChain = nil; + } + } + output = nil; } - (void)pause diff --git a/Audio/Chain/InputNode.h b/Audio/Chain/InputNode.h index d54d7fe42..03c5f9e25 100644 --- a/Audio/Chain/InputNode.h +++ b/Audio/Chain/InputNode.h @@ -28,6 +28,8 @@ BOOL shouldSeek; long seekFrame; + + BOOL observersAdded; Semaphore *exitAtTheEndOfTheStream; } diff --git a/Audio/Chain/InputNode.m b/Audio/Chain/InputNode.m index f63437bb4..cbdadaa4e 100644 --- a/Audio/Chain/InputNode.m +++ b/Audio/Chain/InputNode.m @@ -90,6 +90,8 @@ forKeyPath:@"metadata" options:(NSKeyValueObservingOptionNew) context:NULL]; + + observersAdded = YES; } - (void)observeValueForKeyPath:(NSString *)keyPath @@ -219,8 +221,19 @@ - (void)removeObservers { - [decoder removeObserver:self forKeyPath:@"properties"]; - [decoder removeObserver:self forKeyPath:@"metadata"]; + if (observersAdded) + { + [decoder removeObserver:self forKeyPath:@"properties"]; + [decoder removeObserver:self forKeyPath:@"metadata"]; + observersAdded = NO; + } +} + +- (void)setShouldContinue:(BOOL)s +{ + [super setShouldContinue:s]; + if (!s) + [self removeObservers]; } - (void)dealloc diff --git a/Audio/Chain/OutputNode.m b/Audio/Chain/OutputNode.m index 9f3903577..538448ede 100644 --- a/Audio/Chain/OutputNode.m +++ b/Audio/Chain/OutputNode.m @@ -137,6 +137,7 @@ - (void)close { [output stop]; + output = nil; } - (void)setVolume:(double) v diff --git a/Audio/CogPluginMulti.m b/Audio/CogPluginMulti.m index 169a088bf..66dcce06d 100644 --- a/Audio/CogPluginMulti.m +++ b/Audio/CogPluginMulti.m @@ -108,10 +108,11 @@ NSArray * sortClassesByPriority(NSArray * theClasses) - (void)close { if ( theDecoder != nil ) { - [theDecoder close]; for (NSDictionary *obsItem in cachedObservers) { [theDecoder removeObserver:[obsItem objectForKey:@"observer"] forKeyPath:[obsItem objectForKey:@"keyPath"]]; } + [cachedObservers removeAllObjects]; + [theDecoder close]; theDecoder = nil; } } diff --git a/Audio/Output/OutputCoreAudio.h b/Audio/Output/OutputCoreAudio.h index 0942d41d2..ef4f49b92 100644 --- a/Audio/Output/OutputCoreAudio.h +++ b/Audio/Output/OutputCoreAudio.h @@ -46,6 +46,7 @@ atomic_long bytesHdcdSustained; BOOL listenerapplied; + BOOL observersapplied; float volume; diff --git a/Audio/Output/OutputCoreAudio.m b/Audio/Output/OutputCoreAudio.m index af251097e..4d29201ff 100644 --- a/Audio/Output/OutputCoreAudio.m +++ b/Audio/Output/OutputCoreAudio.m @@ -174,10 +174,7 @@ static OSStatus renderCallback( void *inRefCon, AudioUnitRenderActionFlags *ioAc #ifdef OUTPUT_LOG _logFile = fopen("/tmp/CogAudioLog.raw", "wb"); #endif - - [[NSUserDefaultsController sharedUserDefaultsController] addObserver:self forKeyPath:@"values.outputDevice" options:0 context:NULL]; - [[NSUserDefaultsController sharedUserDefaultsController] addObserver:self forKeyPath:@"values.GraphicEQenable" options:0 context:NULL]; - } + } return self; } @@ -704,6 +701,10 @@ default_device_changed(AudioObjectID inObjectID, UInt32 inNumberAddresses, const [_au allocateRenderResourcesAndReturnError:&err]; + [[NSUserDefaultsController sharedUserDefaultsController] addObserver:self forKeyPath:@"values.outputDevice" options:0 context:NULL]; + [[NSUserDefaultsController sharedUserDefaultsController] addObserver:self forKeyPath:@"values.GraphicEQenable" options:0 context:NULL]; + observersapplied = YES; + return (err == nil); } @@ -732,6 +733,12 @@ default_device_changed(AudioObjectID inObjectID, UInt32 inNumberAddresses, const - (void)stop { + stopInvoked = YES; + if (observersapplied) { + observersapplied = NO; + [[NSUserDefaultsController sharedUserDefaultsController] removeObserver:self forKeyPath:@"values.outputDevice"]; + [[NSUserDefaultsController sharedUserDefaultsController] removeObserver:self forKeyPath:@"values.GraphicEQenable"]; + } if (stopNext && started && !paused) { while (![[outputController buffer] isEmpty]) { [writeSemaphore signal]; @@ -743,7 +750,6 @@ default_device_changed(AudioObjectID inObjectID, UInt32 inNumberAddresses, const stopNext = NO; [self signalEndOfStream]; } - stopInvoked = YES; stopping = YES; paused = NO; [writeSemaphore signal]; @@ -782,14 +788,13 @@ default_device_changed(AudioObjectID inObjectID, UInt32 inNumberAddresses, const _logFile = NULL; } #endif + outputController = nil; } - (void)dealloc { - [self stop]; - - [[NSUserDefaultsController sharedUserDefaultsController] removeObserver:self forKeyPath:@"values.outputDevice"]; - [[NSUserDefaultsController sharedUserDefaultsController] removeObserver:self forKeyPath:@"values.GraphicEQenable"]; + if (!stopInvoked) + [self stop]; } - (void)pause