Conversation
|
📄 Documentation for this branch is available at: https://ncar.github.io/musica/branch/deletion/ |
There was a problem hiding this comment.
Pull request overview
This PR adds manual memory management for JavaScript objects created via Emscripten bindings. According to Emscripten documentation, JavaScript wrappers for C++ objects must be manually deleted to prevent memory leaks.
Changes:
- Added
delete()methods to MICM and State classes to clean up native WASM objects - Updated all test cases in
micm.test.jsto calldelete()on MICM and State instances after use
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
javascript/micm/micm.js |
Added delete() method to clean up _nativeMICM object |
javascript/micm/state.js |
Added delete() method to clean up _nativeState object |
javascript/tests/unit/micm.test.js |
Added cleanup calls for all MICM and State instances created in tests |
Comments suppressed due to low confidence (2)
javascript/micm/state.js:48
- The VectorConditions object created here is never deleted, causing a memory leak. According to Emscripten documentation, all native objects must be manually deleted. Consider calling
vec.delete()after line 78 where it's used.
const vec = new backend.VectorConditions();
javascript/micm/state.js:74
- The Condition objects created in this loop are never deleted, causing a memory leak. Each Condition object should be deleted after being added to the vector, or they should be deleted when the VectorConditions is deleted (if ownership is transferred).
const cond = new backend.Condition(T, P, rho);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #738 +/- ##
==========================================
- Coverage 75.80% 74.92% -0.89%
==========================================
Files 108 109 +1
Lines 7974 8080 +106
==========================================
+ Hits 6045 6054 +9
- Misses 1929 2026 +97
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <[email protected]>
Javascript objects must manually be deleted. This adds that to our classes and tests