ringcentral / RingCentral.Net

RingCentral SDK for .NET
MIT License
19 stars 26 forks source link

RestClient.Dispose() has an async code problem #61

Closed zirplsoftware closed 6 months ago

zirplsoftware commented 8 months ago

This is the RestClient.Dispose() code:

        public async void Dispose()
        {
            await Revoke();
        }

Marking any method with "async void" instead of "async Task" will cause the thread that called it to move on before the execution of Dispose commences. So if you are disposing of this in a using statement, Revoke is not called until after the using block has already completed. In addition, .Net Frameworks's garbage collection has been altered in ways that I could only speculate on.

Although there is an IAsyncDisposable in .Net Standard 2.1, .Net Standard 2.0 does not have this. This should be how it is handled:

public class MyAsyncDisposableResource : IDisposable
{
    // Synchronous Dispose method
    public void Dispose()
    {
        DisposeAsync().GetAwaiter().GetResult();
    }

    // Asynchronous cleanup logic
    public async Task DisposeAsync()
    {
        // Perform asynchronous cleanup operations here
        await Task.Delay(1000); // Example delay to simulate async operation
    }
}

There is an additional issue with the Dispose method in that Revoke is not always desired to be called, as it revokes the refresh token that the calling code may have counted on to be able to use later. I submitted this as #62 and both can be solved at once.

tylerlong commented 7 months ago

Thank you for submitting this issue. From C# programming perspective, you are correct. But the dispose is just to revoke the existing token, it doesn't need to be very time precise. Does it matter for the token revocation to be completed before the using statement finished or a little while (milliseconds) after? I don't think so. Your business logic shouldn't depend on those milliseconds difference.

I will leave this issue open for a while to see if there are any real issues caused by the current design.