Memory Leak in OData Linq Extension

[chadcampbell]

Hello,

The OData Linq Extension has a memory leak. A ServiceContainer object is getting created, however, it's never disposed. This causes CPU usage to spike over time.

I created a fix. I wanted to submit a pull request, however, I was unable to do so on this repo. The changes I made are limited. It would be super helpful if these changes could be integrated and a new NuGet package get published. The changes are in two files:

Community.Data.OData.Linq/ODataQuery.cs

Added IDisposable interface to the class. public class ODataQuery<T> : IQueryable<T>, IDisposable

Implemented the Dispose method.

public void Dispose()
{
  if (this.ServiceProvider != null)
  {
    if (this.ServiceProvider is IDisposable)
    {
      ((IDisposable)(this.ServiceProvider)).Dispose();
    }
  }
}

Community.Data.OData.Linq.xTests

Added test of Dispose method

[Fact]
public void Disposable()
{
  // Generate the list of items to query
  var items = new List<TestItem>();
  items.Add(new TestItem() { Id = Guid.NewGuid(), Name = "Test", Number = 1 });
  items.Add(new TestItem() { Id = Guid.NewGuid(), Name = "Another", Number = 2 });

  var odata = items.AsQueryable().OData();
  var filteredItems = odata.Filter("Number eq 2");
  odata.Dispose();                                        // Test that the OData object is being torn down

  Assert.Equal(1, filteredItems.Count());
}

Added TestItem class for testing needs

/// <summary>
/// A utility class for use in ODataTests
/// </summary>
public class TestItem
{
  public Guid Id { get; set; }
  public string Name { get; set; }
  public int Number { get; set; }
}

[IharYakimush]

Hi @chadcampbell ,

ServiceProvider is public, so you can dispose it if you want as a workaround.

Regarding implementing IDisposable for the ODataQuery<T> to dispose ServiceProvider: I need some time to investigate if it really needed and find the best approach to do that. IQuerable implementing IDisposable is somthing uncommon.

So keeping this issue open for now.

[IharYakimush]

Also memory leak and CPU usage spike is not the same, so could you please provide some details about how you identified an issue. If you have some sample code to reproduce it would be great.

[chadcampbell]

@IharYakimush Thank you for reviewing. I mistakenly said CPU usage spike.