-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix two sources of references to detached rows #2392
Conversation
@atmgrifter00 would you please buddy? |
@m-akinc thinking back historically on changes we have made to FAST Foundation and then having discovered:
For the two PRs that are being merged into FAST archives: microsoft/fast#6960
|
@rajsite I think that has been done for the older fix, but not for the memory leak issue. The memory leak fix ought to have very limited ability to affect anything in an unexpected way, unless |
Pull Request
🤨 Rationale
While investigating a memory leak issue reported in AzDO, we found a couple places in the Nimble table code that wasn't cleaning up observers/notifiers, causing us to hold references to disconnected row elements.
There is also a FAST bug that contributes to the leak (which is getting fixed), so the leak will still not be fixed until this PR and the FAST one are both in.
👩💻 Implementation
🧪 Testing
Verified (in Chrome dev tools) that after these changes, detached rows are no longer referenced via the columns or keyboard navigation manager.
✅ Checklist