I am creating a Xamarin Forms application, and I am using the Xamarin Profiler to show that I have a memory leak. I have tracked the memory leak down to where it is happening, but I can't understand WHY it is happening.
I have a class (we will call it MyClass for now). And that class is using a Timer to call a service once every second. That service makes a REST call to retrieve a bunch of information, and then serializes the results back into an object....
MyClass:
public class MyClass : ContentPage
private readonly IMyService myService;
public MyClass() : base()
{
// I am getting the service provider from my Application. I would
// normally not do it this way, but MyClass is actually a parent
// of all my ContentPages so DI isn't working by passing the class in.
// Meaning, I may have "public partial class MyPage : MyClass". So not
// sure if that is having an impact?
myService = ((App)App.Current)
.serviceProvider
.GetRequiredService<IMyService>();
}
protected override async void OnAppearing()
{
StartTimer();
}
private void StartTimer()
{
Device.StartTimer(TimeSpan.FromSeconds(1), () =>
{
Task t = Task.Run(async() =>
{
//--- everytime I call myService.GetSystemStatus(), my allocated memory continues to rise
MyResponse response = await myService.GetSystemStatus();
Device.BeginInvokeOnMainThread(() =>
{
// update the UI here...
});
});
t.Wait();
t.Dispose();
StartTimer();
return false;
});
}
MyService (Singleton):
public class MyService : IMyService
{
private readonly IMyHttpClientFactory httpClientFactory;
public MyService(IMyHttpClientFactory httpClientFactory)
{
this.httpClientFactory = httpClientFactory;
}
public async Task<MyResponse> GetSystemStatus()
{
return await httpClientFactory.Create().GetAsync<MyResponse>(
"http://example.com/api/status"
);
}
}
MyHttpClientFactory (Singleton):
public class MyHttpClientFactory : IMyHttpClientFactory
{
private readonly IServiceProvider _serviceProvider;
public MyHttpClientFactory(IServiceProvider serviceProvider)
{
_serviceProvider = serviceProvider;
}
public MyHttpClient Create()
{
return _serviceProvider.GetRequiredService<MyHttpClient>();
}
}
MyHttpClient:
public class MyHttpClient : IMyHttpClient, IDisposable
{
private HttpClient _httpClient;
public MyHttpClient ()
{
_httpClient = new HttpClient();
_httpClient.Timeout = TimeSpan.FromSeconds(10);
}
public async Task<T> GetAsync<T>(string url) where T : new()
{
string s = await GetStringAsync(url);
return JsonConvert.DeserializeObject<T>(s);
}
public async Task<string> GetStringAsync(string url)
{
using (var response = await _httpClient.GetAsync(url))
{
response.EnsureSuccessStatusCode();
return await response.Content.ReadAsStringAsync();
}
}
}
My services are defined as follows:
public partial class App : Application
public ServiceProvider serviceProvider;
public App()
{
IServiceCollection services = new ServiceCollection();
ConfigureServices(services);
serviceProvider = services.BuildServiceProvider();
InitializeComponent();
}
private void ConfigureServices(IServiceCollection services)
{
services.AddHttpClient<MyHttpClient>("MyHttpClient", x =>
{
x.Timeout = TimeSpan.FromSeconds(5);
});
services.AddSingleton<IMyHttpClientFactory, MyHttpClientFactory>();
services.AddSingleton<IMyService, MyService>();
}
}
Best I can tell, the memory is going up because I am referencing the DI MyService inside a separate thread. But I am not sure if this is the reason or if there is something else that would be causing the leak?
Any advice would be greatly appreciated!!!
Thanks!
CodePudding user response:
From what I understand from your code and your comments, it looks like you're looping by calling StartTimer()
inside the Device.StartTimer()
method.
According to the documentation, Device.StartTimer()
is recurring and will occur every X seconds, depending of your interval
parameter.
By removing the call to StartTimer()
(the one between t.Dispose()
and return false
of MyClass.StartTimer
, your code should work as expected and you will not create a new timer every x seconds
CodePudding user response:
What could be the cause of the leak:
Your MyHttpClient
class implements the IDisposable
interface, yet the code to use an instance of this class is not leveraging the disposable nature of the object.
Even though the internal HttpClient
instance is wrapped in a using statement, the MyHttpClient
instance will not be disposed of as you would expect.
// from MyHttpClient class
public async Task<MyResponse> GetSystemStatus()
{
// no using statement here
return await httpClientFactory.Create().GetAsync<MyResponse>(
"http://example.com/api/status"
);
}
// should be:
public async Task<MyResponse> GetSystemStatus()
{
using (var client = await httpClientFactory.Create())
{
return await client.GetAsync<MyResponse>("http://example.com/api/status");
}
}
Another thing to try is to change the location of the resolution of the MyService
instance to inside the Task since this is where it is used. This will allow the task to own the resource, and allow it to be collected when the task is complete.
private void StartTimer()
{
Device.StartTimer(TimeSpan.FromSeconds(1), () =>
{
Task t = Task.Run(async() =>
{
// resolve the service here
myService = ((App)App.Current)
.serviceProvider
.GetRequiredService<IMyService>();
MyResponse response = await myService.GetSystemStatus();
Device.BeginInvokeOnMainThread(() =>
{
// update the UI here...
});
});
t.Wait();
t.Dispose();
StartTimer();
return false;
});
}
A couple of additional observations of your code:
In your HttpClientFactory
's Create()
method, you are resolving an instance of your client from the DI container.
Your MyHttpClient
class has a default constructor which means the resolution is not needed since there are no additional dependencies requiring DI support.
Your code could simply return a new MyHttpClient()
instance from the Create()
method without the need for DI.
Your MyHttpClient
also implements the IMyHttpClient
interface, but your factory returns the concrete type. This means you need to either remove the interface as unnecessary or change the return type to be the interface type since the interface is redundant unless it is used.