search

segmentio / Analytics-CSharp

The hassle-free way to add Segment analytics to your C# written app (.Net/Xamarin/Unity).
MIT License
20 stars 8 forks source link

Implement the IDisposable interface on Segment.Analytics.Analytics #113

Open gabor-bocso opened 1 month ago

gabor-bocso commented 1 month ago

Is your feature request related to a problem? Please describe. There is no way to explicit Dispose the Segment.Analytics.Analytics client as it's not implements the IDisposable interface.

Describe the solution you'd like Properly implement the IDisposable pattern, carefully watching for possible open semaphores (avoiding past mistakes) https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose

Describe alternatives you've considered I dont have alternatives as I dont know how it's possible to "throw away" a Segment.Analytics.Analytics client.

Additional context We decided to move away from the old package (https://github.com/segmentio/Analytics.NET) as it's Dispose method is still buggy (the used semaphore is discarded in case a http post is still on the fly, which makes it crash like hell if the timing is right). During the migration we noticed that the old package implemented the IDisposable interface which we used, the new one doesn't.

wenxi-zeng commented 1 month ago

hi @gabor-bocso sorry for the delayed response and thanks for bring this inconsistency to our attention.

can you elaborate more about your use case of having to dispose the analytics object? any memory leak or unmanaged resources having been observed?

the old .NET library disposes semaphore in a brutal way without caring for running threads. this library handles threading in a better way. besides that, it uses SemaphoreSlim instead of Semaphore. to the best of my knowledge, the only unmanaged resources within a SemaphoreSlim is wait handle. so as long as we are not setting the wait handle, we don't have to worry about calling dispose.

are you looking for a way to make sure analytics is terminated completely? we do have a shutdown method to be implemented on our roadmap. we could get this prioritized if that solves your concern.

gabor-bocso commented 3 weeks ago

Hi,

Thanks for the answer, I was wondering if there is a ways to use the client as a transient resource instead of a singleton one, thats why was asking if the IDisposable interface will be implemented or the client is meant to be for singleton usage.

wenxi-zeng commented 3 weeks ago

@gabor-bocso ah I see. yeah, we'll consider make analytics disposable when we add the shutdown function. for now, as a work around, you can make a wrapper of analytics that implements disposable and make analytics = null when dispose.