From 9a7fd25b97a5feb231e0d934cf84c189bbcd32ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20Fazekas?= Date: Mon, 9 Feb 2026 13:54:57 +0100 Subject: [PATCH 1/2] fix: use-after-free in CommandQueue delete methods Remove premature listener deletion from all 6 delete methods (deleteFile, deleteArtboard, deleteViewModelInstance, deleteImage, deleteFont, deleteAudio). The C++ CommandQueue still holds raw pointers to listeners after deletion, causing EXC_BAD_ACCESS when processMessages() invokes callbacks through dangling pointers. Listeners are already cleaned up in dealloc. --- .../CommandQueue/RiveCommandQueue.mm | 61 ------------------- 1 file changed, 61 deletions(-) diff --git a/Source/Experimental/CommandQueue/RiveCommandQueue.mm b/Source/Experimental/CommandQueue/RiveCommandQueue.mm index 4eb85cee..2bf30877 100644 --- a/Source/Experimental/CommandQueue/RiveCommandQueue.mm +++ b/Source/Experimental/CommandQueue/RiveCommandQueue.mm @@ -1169,16 +1169,6 @@ - (uint64_t)loadFile:(nonnull NSData*)data - (void)deleteFile:(uint64_t)file requestID:(uint64_t)requestID { [self executeCommand:^{ - // Clean up the file listener - NSValue* listenerValue = self->_fileListeners[@(file)]; - if (listenerValue) - { - _FileListener* listener = - static_cast<_FileListener*>(listenerValue.pointerValue); - delete listener; - [self->_fileListeners removeObjectForKey:@(file)]; - } - auto handle = reinterpret_cast(file); self->_commandQueue->deleteFile(handle, requestID); }]; @@ -1283,16 +1273,6 @@ - (uint64_t)createArtboardNamed:(NSString*)name - (void)deleteArtboard:(uint64_t)artboard requestID:(uint64_t)requestID { [self executeCommand:^{ - // Remove and release the associated listener - NSValue* listenerValue = self->_artboardListeners[@(artboard)]; - if (listenerValue) - { - _ArtboardListener* listener = - static_cast<_ArtboardListener*>(listenerValue.pointerValue); - delete listener; - [self->_artboardListeners removeObjectForKey:@(artboard)]; - } - auto handle = reinterpret_cast(artboard); self->_commandQueue->deleteArtboard(handle, requestID); }]; @@ -1882,19 +1862,6 @@ - (void)deleteViewModelInstance:(uint64_t)viewModelInstance requestID:(uint64_t)requestID { [self executeCommand:^{ - // Remove and release the associated listener - NSValue* listenerValue = - self->_viewModelInstanceListeners[@(viewModelInstance)]; - if (listenerValue) - { - _ViewModelInstanceListener* listener = - static_cast<_ViewModelInstanceListener*>( - listenerValue.pointerValue); - delete listener; - [self->_viewModelInstanceListeners - removeObjectForKey:@(viewModelInstance)]; - } - auto handle = reinterpret_cast(viewModelInstance); self->_commandQueue->deleteViewModelInstance(handle, requestID); @@ -1964,15 +1931,6 @@ - (uint64_t)decodeImage:(NSData*)data - (void)deleteImage:(uint64_t)renderImage requestID:(uint64_t)requestID { [self executeCommand:^{ - NSValue* listenerValue = self->_renderImageListeners[@(renderImage)]; - if (listenerValue) - { - _RenderImageListener* listener = - static_cast<_RenderImageListener*>(listenerValue.pointerValue); - delete listener; - [self->_renderImageListeners removeObjectForKey:@(renderImage)]; - } - auto handle = reinterpret_cast(renderImage); self->_commandQueue->deleteImage(handle, requestID); }]; @@ -2025,15 +1983,6 @@ - (uint64_t)decodeFont:(NSData*)data - (void)deleteFont:(uint64_t)font requestID:(uint64_t)requestID { [self executeCommand:^{ - NSValue* listenerValue = self->_fontListeners[@(font)]; - if (listenerValue) - { - _FontListener* listener = - static_cast<_FontListener*>(listenerValue.pointerValue); - delete listener; - [self->_fontListeners removeObjectForKey:@(font)]; - } - auto handle = reinterpret_cast(font); self->_commandQueue->deleteFont(handle, requestID); }]; @@ -2086,15 +2035,6 @@ - (uint64_t)decodeAudio:(NSData*)data - (void)deleteAudio:(uint64_t)audio requestID:(uint64_t)requestID { [self executeCommand:^{ - NSValue* listenerValue = self->_audioListeners[@(audio)]; - if (listenerValue) - { - _AudioListener* listener = - static_cast<_AudioListener*>(listenerValue.pointerValue); - delete listener; - [self->_audioListeners removeObjectForKey:@(audio)]; - } - auto handle = reinterpret_cast(audio); self->_commandQueue->deleteAudio(handle, requestID); }]; @@ -2314,7 +2254,6 @@ - (uint64_t)executeCommandWithReturn:(uint64_t (^)(void))commandBlock */ - (void)processMessages { - // Process messages directly since we're already on the main queue _commandQueue->processMessages(); } From b9ea289ad3f5758916071db384fc48b672512eb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20Fazekas?= Date: Tue, 10 Feb 2026 09:31:55 +0100 Subject: [PATCH 2/2] fix: eager listener cleanup via onXxxDeleted callbacks Instead of deferring all listener cleanup to CommandQueue dealloc, clean up listeners as soon as the C++ server confirms deletion via onXxxDeleted callbacks. This prevents listener accumulation when resources are created and destroyed while the Worker stays alive. Dealloc remains as a safety net for bulk teardown (when the Worker itself is released alongside all resources). Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude Co-Authored-By: Happy --- .../CommandQueue/RiveCommandQueue.mm | 169 +++++++++++++++--- 1 file changed, 147 insertions(+), 22 deletions(-) diff --git a/Source/Experimental/CommandQueue/RiveCommandQueue.mm b/Source/Experimental/CommandQueue/RiveCommandQueue.mm index 2bf30877..3068fda4 100644 --- a/Source/Experimental/CommandQueue/RiveCommandQueue.mm +++ b/Source/Experimental/CommandQueue/RiveCommandQueue.mm @@ -27,6 +27,15 @@ NS_ASSUME_NONNULL_BEGIN +@interface RiveCommandQueue () +- (void)_cleanupFileListener:(uint64_t)handle; +- (void)_cleanupArtboardListener:(uint64_t)handle; +- (void)_cleanupViewModelInstanceListener:(uint64_t)handle; +- (void)_cleanupRenderImageListener:(uint64_t)handle; +- (void)_cleanupFontListener:(uint64_t)handle; +- (void)_cleanupAudioListener:(uint64_t)handle; +@end + rive::DataType RiveViewModelInstanceDataTypeToCppType( RiveViewModelInstanceDataType type) { @@ -191,9 +200,11 @@ RiveViewModelInstanceDataType RiveViewModelInstanceDataTypeFromCpp( * @param observer The Objective-C observer that will receive artboard * events. The observer is held as a weak reference to avoid retain cycles. */ - _ArtboardListener(id observer) + _ArtboardListener(id observer, + RiveCommandQueue* queue) { _observer = observer; + _queue = queue; } /** @@ -248,8 +259,12 @@ virtual void onDefaultViewModelInfoReceived( std::string viewModelName, std::string instanceName) override; + virtual void onArtboardDeleted(const rive::ArtboardHandle handle, + uint64_t requestId) override; + private: __weak id _observer; + __weak RiveCommandQueue* _queue; }; } // namespace @@ -305,6 +320,12 @@ virtual void onDefaultViewModelInfoReceived( } } +void _ArtboardListener::onArtboardDeleted(const rive::ArtboardHandle handle, + uint64_t requestId) +{ + [_queue _cleanupArtboardListener:reinterpret_cast(handle)]; +} + // MARK: - Internal File Listener Implementation namespace @@ -328,7 +349,11 @@ virtual void onDefaultViewModelInfoReceived( * The observer is held as a weak reference to avoid retain * cycles. */ - _FileListener(id observer) { _observer = observer; } + _FileListener(id observer, RiveCommandQueue* queue) + { + _observer = observer; + _queue = queue; + } /** * Called when a file loading operation encounters an error. @@ -426,6 +451,7 @@ virtual void onViewModelEnumsListed( private: __weak id _observer; + __weak RiveCommandQueue* _queue; }; } // namespace @@ -459,6 +485,7 @@ virtual void onViewModelEnumsListed( [_observer onFileDeleted:reinterpret_cast(handle) requestID:reinterpret_cast(requestId)]; } + [_queue _cleanupFileListener:reinterpret_cast(handle)]; } void _FileListener::onArtboardsListed(const rive::FileHandle handle, @@ -629,9 +656,11 @@ virtual void onViewModelEnumsListed( * instance events. The observer is held as a weak reference * to avoid retain cycles. */ - _ViewModelInstanceListener(id observer) + _ViewModelInstanceListener(id observer, + RiveCommandQueue* queue) { _observer = observer; + _queue = queue; } /** @@ -683,6 +712,7 @@ virtual void onViewModelListSizeReceived( private: __weak id _observer; + __weak RiveCommandQueue* _queue; }; } // namespace @@ -690,11 +720,15 @@ virtual void onViewModelListSizeReceived( const rive::ViewModelInstanceHandle handle, uint64_t requestId, std::string error) -{} +{ +} void _ViewModelInstanceListener::onViewModelDeleted( const rive::ViewModelInstanceHandle handle, uint64_t requestId) -{} +{ + [_queue _cleanupViewModelInstanceListener:reinterpret_cast( + handle)]; +} void _ViewModelInstanceListener::onViewModelDataReceived( const rive::ViewModelInstanceHandle handle, @@ -735,9 +769,11 @@ virtual void onViewModelListSizeReceived( class _RenderImageListener : public rive::CommandQueue::RenderImageListener { public: - _RenderImageListener(id observer) + _RenderImageListener(id observer, + RiveCommandQueue* queue) { _observer = observer; + _queue = queue; } virtual void onRenderImageDecoded(const rive::RenderImageHandle handle, @@ -752,6 +788,7 @@ virtual void onRenderImageDeleted(const rive::RenderImageHandle handle, private: __weak id _observer; + __weak RiveCommandQueue* _queue; }; } // namespace @@ -785,6 +822,7 @@ virtual void onRenderImageDeleted(const rive::RenderImageHandle handle, [_observer onRenderImageDeleted:reinterpret_cast(handle) requestID:requestId]; } + [_queue _cleanupRenderImageListener:reinterpret_cast(handle)]; } namespace @@ -792,7 +830,11 @@ virtual void onRenderImageDeleted(const rive::RenderImageHandle handle, class _FontListener : public rive::CommandQueue::FontListener { public: - _FontListener(id observer) { _observer = observer; } + _FontListener(id observer, RiveCommandQueue* queue) + { + _observer = observer; + _queue = queue; + } virtual void onFontDecoded(const rive::FontHandle handle, uint64_t requestId) override; @@ -806,6 +848,7 @@ virtual void onFontDeleted(const rive::FontHandle handle, private: __weak id _observer; + __weak RiveCommandQueue* _queue; }; } // namespace @@ -839,6 +882,7 @@ virtual void onFontDeleted(const rive::FontHandle handle, [_observer onFontDeleted:reinterpret_cast(handle) requestID:requestId]; } + [_queue _cleanupFontListener:reinterpret_cast(handle)]; } namespace @@ -846,7 +890,11 @@ virtual void onFontDeleted(const rive::FontHandle handle, class _AudioListener : public rive::CommandQueue::AudioSourceListener { public: - _AudioListener(id observer) { _observer = observer; } + _AudioListener(id observer, RiveCommandQueue* queue) + { + _observer = observer; + _queue = queue; + } virtual void onAudioSourceDecoded(const rive::AudioSourceHandle handle, uint64_t requestId) override; @@ -860,6 +908,7 @@ virtual void onAudioSourceDeleted(const rive::AudioSourceHandle handle, private: __weak id _observer; + __weak RiveCommandQueue* _queue; }; } // namespace @@ -894,6 +943,7 @@ virtual void onAudioSourceDeleted(const rive::AudioSourceHandle handle, [_observer onAudioSourceDeleted:reinterpret_cast(handle) requestID:requestId]; } + [_queue _cleanupAudioListener:reinterpret_cast(handle)]; } /** @@ -1137,7 +1187,7 @@ - (uint64_t)loadFile:(nonnull NSData*)data { return [self executeCommandWithReturn:^uint64_t { // Create a new listener for this specific observer - auto listener = std::make_unique<_FileListener>(observer); + auto listener = std::make_unique<_FileListener>(observer, self); const uint8_t* bytes = static_cast(data.bytes); size_t length = data.length; @@ -1228,7 +1278,7 @@ - (uint64_t)createDefaultArtboardFromFile:(uint64_t)fileHandle { return [self executeCommandWithReturn:^uint64_t { // Create a new listener for this specific observer - auto listener = std::make_unique<_ArtboardListener>(observer); + auto listener = std::make_unique<_ArtboardListener>(observer, self); auto handle = reinterpret_cast(fileHandle); rive::ArtboardHandle artboardHandle = @@ -1251,7 +1301,7 @@ - (uint64_t)createArtboardNamed:(NSString*)name { return [self executeCommandWithReturn:^uint64_t { // Create a new listener for this specific observer - auto listener = std::make_unique<_ArtboardListener>(observer); + auto listener = std::make_unique<_ArtboardListener>(observer, self); auto handle = reinterpret_cast(fileHandle); auto stdName = std::string([name UTF8String]); @@ -1488,7 +1538,7 @@ - (void)draw:(uint64_t)drawKey callback:(void (^)(void*))callback { return [self executeCommandWithReturn:^uint64_t { // Create a new listener for this specific observer - auto listener = std::make_unique<_ViewModelInstanceListener>(observer); + auto listener = std::make_unique<_ViewModelInstanceListener>(observer, self); rive::ViewModelInstanceHandle handle = self->_commandQueue->instantiateBlankViewModelInstance( @@ -1516,7 +1566,7 @@ - (void)draw:(uint64_t)drawKey callback:(void (^)(void*))callback { return [self executeCommandWithReturn:^uint64_t { // Create a new listener for this specific observer - auto listener = std::make_unique<_ViewModelInstanceListener>(observer); + auto listener = std::make_unique<_ViewModelInstanceListener>(observer, self); auto stdName = std::string([viewModelName UTF8String]); rive::ViewModelInstanceHandle handle = @@ -1546,7 +1596,7 @@ - (void)draw:(uint64_t)drawKey callback:(void (^)(void*))callback { return [self executeCommandWithReturn:^uint64_t { // Create a new listener for this specific observer - auto listener = std::make_unique<_ViewModelInstanceListener>(observer); + auto listener = std::make_unique<_ViewModelInstanceListener>(observer, self); rive::ViewModelInstanceHandle handle = self->_commandQueue->instantiateDefaultViewModelInstance( @@ -1574,7 +1624,7 @@ - (void)draw:(uint64_t)drawKey callback:(void (^)(void*))callback { return [self executeCommandWithReturn:^uint64_t { // Create a new listener for this specific observer - auto listener = std::make_unique<_ViewModelInstanceListener>(observer); + auto listener = std::make_unique<_ViewModelInstanceListener>(observer, self); auto stdName = std::string([viewModelName UTF8String]); rive::ViewModelInstanceHandle handle = @@ -1602,7 +1652,7 @@ - (uint64_t)createViewModelInstanceNamed:(NSString*)instanceName requestID:(uint64_t)requestID { return [self executeCommandWithReturn:^uint64_t { - auto listener = std::make_unique<_ViewModelInstanceListener>(observer); + auto listener = std::make_unique<_ViewModelInstanceListener>(observer, self); auto stdInstanceName = std::string([instanceName UTF8String]); rive::ViewModelInstanceHandle handle = @@ -1630,7 +1680,7 @@ - (uint64_t)createViewModelInstanceNamed:(NSString*)instanceName requestID:(uint64_t)requestID { return [self executeCommandWithReturn:^uint64_t { - auto listener = std::make_unique<_ViewModelInstanceListener>(observer); + auto listener = std::make_unique<_ViewModelInstanceListener>(observer, self); auto stdViewModelName = std::string([viewModelName UTF8String]); auto stdInstanceName = std::string([instanceName UTF8String]); @@ -1910,7 +1960,7 @@ - (uint64_t)decodeImage:(NSData*)data { return [self executeCommandWithReturn:^uint64_t { auto renderImageListener = - std::make_unique<_RenderImageListener>(listener); + std::make_unique<_RenderImageListener>(listener, self); const uint8_t* bytes = static_cast(data.bytes); size_t length = data.length; @@ -1962,7 +2012,7 @@ - (uint64_t)decodeFont:(NSData*)data requestID:(uint64_t)requestID { return [self executeCommandWithReturn:^uint64_t { - auto fontListener = std::make_unique<_FontListener>(listener); + auto fontListener = std::make_unique<_FontListener>(listener, self); const uint8_t* bytes = static_cast(data.bytes); size_t length = data.length; @@ -2014,7 +2064,7 @@ - (uint64_t)decodeAudio:(NSData*)data requestID:(uint64_t)requestID { return [self executeCommandWithReturn:^uint64_t { - auto audioListener = std::make_unique<_AudioListener>(listener); + auto audioListener = std::make_unique<_AudioListener>(listener, self); const uint8_t* bytes = static_cast(data.bytes); size_t length = data.length; @@ -2067,7 +2117,7 @@ - (void)removeGlobalAudioAsset:(NSString*)name requestID:(uint64_t)requestID { return [self executeCommandWithReturn:^uint64_t { // Create a new listener for this specific observer - auto listener = std::make_unique<_ViewModelInstanceListener>(observer); + auto listener = std::make_unique<_ViewModelInstanceListener>(observer, self); auto stdPath = std::string([path UTF8String]); auto vmiHandle = reinterpret_cast( @@ -2093,7 +2143,7 @@ - (void)removeGlobalAudioAsset:(NSString*)name requestID:(uint64_t)requestID requestID:(uint64_t)requestID { return [self executeCommandWithReturn:^uint64_t { - auto listener = std::make_unique<_ViewModelInstanceListener>(observer); + auto listener = std::make_unique<_ViewModelInstanceListener>(observer, self); auto stdPath = std::string([path UTF8String]); auto vmiHandle = reinterpret_cast( @@ -2257,6 +2307,81 @@ - (void)processMessages _commandQueue->processMessages(); } +#pragma mark - Listener Cleanup + +- (void)_cleanupFileListener:(uint64_t)handle +{ + NSValue* listenerValue = _fileListeners[@(handle)]; + if (listenerValue) + { + _FileListener* listener = + static_cast<_FileListener*>(listenerValue.pointerValue); + [_fileListeners removeObjectForKey:@(handle)]; + delete listener; + } +} + +- (void)_cleanupArtboardListener:(uint64_t)handle +{ + NSValue* listenerValue = _artboardListeners[@(handle)]; + if (listenerValue) + { + _ArtboardListener* listener = + static_cast<_ArtboardListener*>(listenerValue.pointerValue); + [_artboardListeners removeObjectForKey:@(handle)]; + delete listener; + } +} + +- (void)_cleanupViewModelInstanceListener:(uint64_t)handle +{ + NSValue* listenerValue = _viewModelInstanceListeners[@(handle)]; + if (listenerValue) + { + _ViewModelInstanceListener* listener = + static_cast<_ViewModelInstanceListener*>( + listenerValue.pointerValue); + [_viewModelInstanceListeners removeObjectForKey:@(handle)]; + delete listener; + } +} + +- (void)_cleanupRenderImageListener:(uint64_t)handle +{ + NSValue* listenerValue = _renderImageListeners[@(handle)]; + if (listenerValue) + { + _RenderImageListener* listener = + static_cast<_RenderImageListener*>(listenerValue.pointerValue); + [_renderImageListeners removeObjectForKey:@(handle)]; + delete listener; + } +} + +- (void)_cleanupFontListener:(uint64_t)handle +{ + NSValue* listenerValue = _fontListeners[@(handle)]; + if (listenerValue) + { + _FontListener* listener = + static_cast<_FontListener*>(listenerValue.pointerValue); + [_fontListeners removeObjectForKey:@(handle)]; + delete listener; + } +} + +- (void)_cleanupAudioListener:(uint64_t)handle +{ + NSValue* listenerValue = _audioListeners[@(handle)]; + if (listenerValue) + { + _AudioListener* listener = + static_cast<_AudioListener*>(listenerValue.pointerValue); + [_audioListeners removeObjectForKey:@(handle)]; + delete listener; + } +} + @end NS_ASSUME_NONNULL_END