[runtime] Fix memory leak with BlockLiteral descriptors. Fixes #20503. #20556
Conversation
…n#20503. We're using two different functions to atomically decrement a reference count, the native `atomic_fetch_sub` and the managed `Interlocked.Decrement`. Unfortunately the return value is not the same: `atomic_fetch_sub` returns the original value before the subtraction, while `Interlocked.Decrement` returns the subtracted value, while our code assumed the functions behaved the same. This resulted in a memory leak, because we'd incorrectly expect `0` to be returned from `atomic_fetch_sub` when the reference count reaches zero, and thus not detect when the descriptor a block should be freed. The fix is to update the expected return value from `atomic_fetch_sub` to be `1` instead of `0`. Fixes xamarin#20503.
📚 [PR Build] Artifacts 📚Packages generatedView packagesPipeline on Agent |
💻 [PR Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [PR Build] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [PR Build] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11) passed 💻✅ All tests on macOS M1 - Mac Big Sur (11) passed. Pipeline on Agent |
💻 [CI Build] Windows Integration Tests passed 💻✅ All Windows Integration Tests passed. Pipeline on Agent |
✅ API diff for current PR / commitLegacy Xamarin (No breaking changes)
NET (empty diffs)
✅ API diff vs stableLegacy Xamarin (No breaking changes).NET (No breaking changes)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 131 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
|
/sudo backport release/8.0.1xx-xcode15.1 |
|
I couldn't create a backport to release/8.0.1xx-xcode15.1 for you. :( Please check the Action logs for more details. |
…al descriptors. Fixes #20503. (#20562) We're using two different functions to atomically decrement a reference count, the native `atomic_fetch_sub` and the managed `Interlocked.Decrement`. Unfortunately the return value is not the same: `atomic_fetch_sub` returns the original value before the subtraction, while `Interlocked.Decrement` returns the subtracted value, while our code assumed the functions behaved the same. This resulted in a memory leak, because we'd incorrectly expect `0` to be returned from `atomic_fetch_sub` when the reference count reaches zero, and thus not detect when the descriptor a block should be freed. The fix is to update the expected return value from `atomic_fetch_sub` to be `1` instead of `0`. Fixes #20503. Backport of #20556. --------- Co-authored-by: Alex Soto <alex@alexsoto.me>
We're using two different functions to atomically decrement a reference count,
the native
atomic_fetch_suband the managedInterlocked.Decrement.Unfortunately the return value is not the same:
atomic_fetch_subreturns theoriginal value before the subtraction, while
Interlocked.Decrementreturnsthe subtracted value, while our code assumed the functions behaved the same.
This resulted in a memory leak, because we'd incorrectly expect
0to bereturned from
atomic_fetch_subwhen the reference count reaches zero, andthus not detect when the descriptor a block should be freed.
The fix is to update the expected return value from
atomic_fetch_subto be1instead of0.Fixes #20503.