feat: download stop logic on component unmount#16
feat: download stop logic on component unmount#16harryfrzz wants to merge 10 commits intocactus-compute:mainfrom
Conversation
feat: added stop download logic to stop model downloads when the component unmounts
There was a problem hiding this comment.
Pull request overview
This PR adds functionality to stop model downloads when components unmount, preventing resource leaks and unnecessary network activity. The implementation adds a stopDownload method across the TypeScript API, native iOS/Android implementations, and C++ bridge code.
Key Changes
- Added
stopDownloadmethod toCactusFileSystem,CactusLM, andCactusSTTclasses - Implemented cleanup logic in
useCactusLManduseCactusSTThooks to callstopDownloadon unmount - Updated native iOS (Swift) and Android (Kotlin) implementations to cancel active downloads
- Dependency updates in yarn.lock (various package version bumps)
Reviewed changes
Copilot reviewed 15 out of 42 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/specs/CactusFileSystem.nitro.ts | Added stopDownload method to FileSystem spec |
| src/native/CactusFileSystem.ts | Exposed stopDownload wrapper method |
| src/classes/CactusLM.ts | Added stopDownload method to CactusLM class |
| src/classes/CactusSTT.ts | Added stopDownload method to CactusSTT class |
| src/hooks/useCactusLM.ts | Added cleanup call to stopDownload on unmount |
| src/hooks/useCactusSTT.ts | Added cleanup call to stopDownload on unmount |
| ios/HybridCactusFileSystem.swift | iOS implementation with URLSession task cancellation (contains syntax error) |
| android/.../HybridCactusFileSystem.kt | Android implementation with connection cancellation (has race condition) |
| nitrogen/generated/**/* | Auto-generated bridge code for new method |
| cpp/HybridCactusUtil.* | Updated to use std::variant instead of std::optional for null handling |
| yarn.lock | Dependency updates (various packages) |
| example/src/PerformanceScreen.tsx | Example usage of stopDownload in cleanup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| override fun stopDownload(model: String): Promise<Unit> { | ||
| return Promise.async { | ||
| if (activeConnection != null) { | ||
| isCancelled = true | ||
| activeConnection?.disconnect() | ||
| activeConnection = null |
There was a problem hiding this comment.
Race condition: The activeConnection and isCancelled variables are marked @Volatile but are not properly synchronized. The check-then-act pattern in stopDownload (lines 177-180) is not atomic. Between checking activeConnection != null and calling disconnect(), another thread could set it to null. Consider using synchronized blocks or a lock to ensure thread safety.
| } | ||
| } | ||
|
|
||
| private var activeTask: URLSessionDownloadTask? |
There was a problem hiding this comment.
Race condition: The activeTask property is not thread-safe. Multiple concurrent calls to downloadModel could overwrite each other's tasks, and stopDownload could cancel the wrong download. Consider using a dictionary keyed by model name to track multiple downloads, or add synchronization to prevent concurrent downloads of the same model.
refactor: code errors fix
chore: dependency changes
1 participant
No description provided.