csharp icon indicating copy to clipboard operation
csharp copied to clipboard

Possible improvement for Watcher.Dispose()

Open mikhail-barg opened this issue 1 year ago • 1 comments

Hi!

I think that current implementation of Watcher.Dispose() is a bit lacking:

https://github.com/kubernetes-client/csharp/blob/541abb00bd1472e86b37863822cce5415613806b/src/KubernetesClient/Watcher.cs#L217-L229

specifically between _cts?.Cancel() and _cts?.Dispose() calls, we should await for the _watcherLoop task to finish. Otherwise it might attempt to access the Token of already disposed _cts, which is generally undesirable.

Also possible exception would be unobserved, which in turn may be a problem for applications with strict policies on unobserved exceptions.

What do you think? Will you consider a PR like this:

protected virtual void Dispose(bool disposing)
{
    if (!disposedValue)
    {
        if (disposing)
        {
            _cts.Cancel();
            try 
            {
                _watcherLoop.GetAwaiter().GetResult();
            }
            catch 
            {
                //consume possible exception
            }
            _cts.Dispose();
        }

        disposedValue = true;
    }
}

mikhail-barg avatar Oct 02 '24 21:10 mikhail-barg