Skip to content

bug: Fixes some correctness bug with ContextWatchdog#1688

Merged
SpencerTorres merged 4 commits intomainfrom
kavirajk/context-watchdog-bug
Oct 22, 2025
Merged

bug: Fixes some correctness bug with ContextWatchdog#1688
SpencerTorres merged 4 commits intomainfrom
kavirajk/context-watchdog-bug

Conversation

@kavirajk
Copy link
Copy Markdown
Contributor

Summary

  1. We don't need infinite for loop to call the callback once context timeout
  2. Currently callback will be called multiple times even after context is cancelled. This PR fixes and block the behavior with tests to make sure callback is called only once.
  3. Make sure the goroutine started by watchdog exits at all cases without any leaking

Checklist

Delete items not relevant to your PR:

1. We don't need infinite for loop to call the callback once context
   timeout
2. Currently callback will be called multiple times even after context
   is cancelled. This PR fixes and block the behavior with tests to make
   sure callback is called only once.
3. Make sure the goroutine started by watchdog exits at all cases
   without any leaking

Signed-off-by: Kaviraj <[email protected]>
As synctest is not part of standard library (without EXPERIMENT flags).
The tests would even fail during build. Hence the build tags

Signed-off-by: Kaviraj <[email protected]>
@kavirajk kavirajk marked this pull request as ready for review October 16, 2025 12:53
@@ -0,0 +1,89 @@
//go:build go1.25
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we removed once we upgrade the Go tool chain to v1.25.
#1689

Copy link
Copy Markdown
Member

@SpencerTorres SpencerTorres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad this has test coverage 👌

@SpencerTorres SpencerTorres merged commit a35898e into main Oct 22, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants