kubescape / storage

Apache License 2.0
1 stars 6 forks source link

Fix deadlocks and memory leaks from the watcher/watchDispatcher implementation. #129

Closed ttimonen closed 3 weeks ago

ttimonen commented 1 month ago

There's relatively verbose analysis in the comments on why the watch.Interface is quite convoluted to implement in a way that doesn't deadlock. In particular adhering to both the Hyrum's law version of the API, and the documented version of the API is highly impractical.

Also simplify the testing implementation and add test-cases for the deadlocks.

Possibly controversal part: The context lifetime propagation in the Watch() call. The API expectation on that is not obvious.

The changes are probably easiest to read one commit at a time, since they are quite self-contained.

matthyx commented 1 month ago

Thanks for your PR @ttimonen I'm going away for a week and will probably need a few days to fully comprehend your changes... stay tuned!

matthyx commented 3 weeks ago

@ttimonen thanks again for your work, it took me a while to validate it but it works perfectly!