Skip to content

[ntcore] Update InstanceImpl::Destroy() to wait for listener callback completion #8226

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

[ntcore] Update InstanceImpl::Destroy() to wait for listener callback completion #8226

wants to merge 11 commits into from
+160 −9

Conversation

kcooney
Copy link
Contributor

@kcooney kcooney commented Sep 13, 2025 •
edited
Loading

This can add a delay if NetworkTableInstance::Destroy() is called while a
Listener callback is running.

Fixes #8215

@kcooney kcooney requested a review from a team as a code owner September 13, 2025 22:59
@github-actions github-actions bot added the component: ntcore NetworkTables library label Sep 13, 2025
@kcooney kcooney marked this pull request as draft September 13, 2025 23:49
@kcooney kcooney force-pushed the kcooney/fix-8215 branch 4 times, most recently from 2a86bca to 0b3bc5e Compare September 14, 2025 02:11
@kcooney kcooney marked this pull request as ready for review September 14, 2025 02:47
@kcooney kcooney mentioned this pull request Sep 14, 2025
Open
@kcooney kcooney force-pushed the kcooney/fix-8215 branch 5 times, most recently from 8a9fa5c to e622b24 Compare September 24, 2025 03:42
PeterJohnson
PeterJohnson requested changes Sep 24, 2025
View reviewed changes
@calcmogul calcmogul changed the title Update InstanceImpl::Destroy() to wait for listener callback completion [ntcore] Update InstanceImpl::Destroy() to wait for listener callback completion Sep 27, 2025
@kcooney kcooney requested a review from PeterJohnson October 12, 2025 21:38
@kcooney kcooney force-pushed the kcooney/fix-8215 branch 2 times, most recently from 50b899e to e4de8cc Compare October 12, 2025 22:06
kcooney added 10 commits October 17, 2025 18:50
@kcooney
Update InstanceImpl::Destroy() to wait for listener callback completion
ce9f7b6 
This adds a delay of at most 20ms if NetworkTableInstance::Destroy() is
called while a Listener callback is running.
@kcooney
Update ListenerStorage::Stop() to return quickly if a callback was no…
563be27 
…t running
@kcooney
Reduce sleep time in TableListenerTest.DestroyInstanceWhileInCallack()
b141565 
The test appears to be flaky for debug builds.
@kcooney
Make m_shutdown an atomic_bool (to make ubsan happy)
32500e7
@kcooney
Attempt to fix Windows builds
1ec9549
@kcooney
Grab s_mutex before deleting the instance
9c55df5
@kcooney
Call Join() instead of WaitForObject()
9d17c62
@kcooney
Remove m_shutdown and m_inCallback
2b53f4e
@kcooney
Replace InstanceValid() with Matchers (so better failure messages are…
4c4ae5b 
… provided)
@kcooney
Improve message when matchers do not match
30522cb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PeterJohnson PeterJohnson Awaiting requested review from PeterJohnson

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

component: ntcore NetworkTables library

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[java] SIGSEGV from race conditions between Preferences.setNetworkTableInstance() and NetworkTableInstance.close()

2 participants

@kcooney @PeterJohnson