Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix EF Core Memory Leak #3539

Merged
merged 4 commits into from Jul 13, 2020
Merged

Fix EF Core Memory Leak #3539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8959621
Fix EF Core memory leak
barronpm Jul 12, 2020
befd0c7
Remove EF Core Proxies
barronpm Jul 13, 2020
18c03fa
Merge branch 'master' into fix-memoryleak
barronpm Jul 13, 2020
b468ae2
Use AsEnumerable for UserManager.Users
barronpm Jul 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion Jellyfin.Data/Jellyfin.Data.csproj
View file
Open in desktop
Expand Up @@ -21,7 +21,6 @@
<ItemGroup>
<PackageReference Include="Microsoft.EntityFrameworkCore.Relational" Version="3.1.5" />
<PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" Version="3.1.5" />
<PackageReference Include="Microsoft.EntityFrameworkCore.Proxies" Version="3.1.5" />
</ItemGroup>

</Project>
53 changes: 40 additions & 13 deletions Jellyfin.Server.Implementations/Users/UserManager.cs
View file
Open in desktop
Expand Up @@ -23,6 +23,7 @@
using MediaBrowser.Model.Dto;
using MediaBrowser.Model.Events;
using MediaBrowser.Model.Users;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;

namespace Jellyfin.Server.Implementations.Users
Expand Down Expand Up @@ -86,7 +87,18 @@ public class UserManager : IUserManager
public event EventHandler<GenericEventArgs<User>>? OnUserLockedOut;

/// <inheritdoc/>
public IEnumerable<User> Users => _dbProvider.CreateContext().Users;
public IEnumerable<User> Users
{
get
{
using var dbContext = _dbProvider.CreateContext();
return dbContext.Users.Include(user => user.Permissions)
.Include(user => user.Preferences)
.Include(user => user.AccessSchedules)
.Include(user => user.ProfileImage)
.AsEnumerable();
}
}

/// <inheritdoc/>
public IEnumerable<Guid> UsersIds => _dbProvider.CreateContext().Users.Select(u => u.Id);
Expand All @@ -99,7 +111,12 @@ public class UserManager : IUserManager
throw new ArgumentException("Guid can't be empty", nameof(id));
}

return _dbProvider.CreateContext().Users.Find(id);
using var dbContext = _dbProvider.CreateContext();
return dbContext.Users.Include(user => user.Permissions)
.Include(user => user.Preferences)
.Include(user => user.AccessSchedules)
.Include(user => user.ProfileImage)
.FirstOrDefault(user => user.Id == id);
}

/// <inheritdoc/>
Expand All @@ -110,7 +127,13 @@ public class UserManager : IUserManager
throw new ArgumentException("Invalid username", nameof(name));
}

return _dbProvider.CreateContext().Users.AsEnumerable()
using var dbContext = _dbProvider.CreateContext();

return dbContext.Users.Include(user => user.Permissions)
.Include(user => user.Preferences)
.Include(user => user.AccessSchedules)
.Include(user => user.ProfileImage)
.AsEnumerable()
.FirstOrDefault(u => string.Equals(u.Username, name, StringComparison.OrdinalIgnoreCase));
}

Expand All @@ -127,7 +150,7 @@ public async Task RenameUser(User user, string newName)
throw new ArgumentException("Invalid username", nameof(newName));
}

if (user.Username.Equals(newName, StringComparison.Ordinal))
if (user.Username.Equals(newName, StringComparison.OrdinalIgnoreCase))
{
throw new ArgumentException("The new and old names must be different.");
}
Expand All @@ -149,15 +172,15 @@ public async Task RenameUser(User user, string newName)
/// <inheritdoc/>
public void UpdateUser(User user)
{
var dbContext = _dbProvider.CreateContext();
using var dbContext = _dbProvider.CreateContext();
dbContext.Users.Update(user);
dbContext.SaveChanges();
}

/// <inheritdoc/>
public async Task UpdateUserAsync(User user)
{
var dbContext = _dbProvider.CreateContext();
await using var dbContext = _dbProvider.CreateContext();
dbContext.Users.Update(user);

await dbContext.SaveChangesAsync().ConfigureAwait(false);
Expand All @@ -171,7 +194,7 @@ public User CreateUser(string name)
throw new ArgumentException("Usernames can contain unicode symbols, numbers (0-9), dashes (-), underscores (_), apostrophes ('), and periods (.)");
}

var dbContext = _dbProvider.CreateContext();
using var dbContext = _dbProvider.CreateContext();

// TODO: Remove after user item data is migrated.
var max = dbContext.Users.Any() ? dbContext.Users.Select(u => u.InternalId).Max() : 0;
Expand All @@ -194,8 +217,12 @@ public User CreateUser(string name)
/// <inheritdoc/>
public void DeleteUser(Guid userId)
{
var dbContext = _dbProvider.CreateContext();
var user = dbContext.Users.Find(userId);
using var dbContext = _dbProvider.CreateContext();
var user = dbContext.Users.Include(u => u.Permissions)
.Include(u => u.Preferences)
.Include(u => u.AccessSchedules)
.Include(u => u.ProfileImage)
.FirstOrDefault(u => u.Id == userId);
if (user == null)
{
throw new ResourceNotFoundException(nameof(userId));
Expand Down Expand Up @@ -380,7 +407,7 @@ public UserDto GetUserDto(User user, string? remoteEndPoint = null)
throw new ArgumentNullException(nameof(username));
}

var user = Users.AsEnumerable().FirstOrDefault(i => string.Equals(username, i.Username, StringComparison.OrdinalIgnoreCase));
var user = Users.FirstOrDefault(i => string.Equals(username, i.Username, StringComparison.OrdinalIgnoreCase));
bool success;
IAuthenticationProvider? authenticationProvider;

Expand Down Expand Up @@ -408,7 +435,7 @@ public UserDto GetUserDto(User user, string? remoteEndPoint = null)

// Search the database for the user again
// the authentication provider might have created it
user = Users.AsEnumerable().FirstOrDefault(i => string.Equals(username, i.Username, StringComparison.OrdinalIgnoreCase));
user = Users.FirstOrDefault(i => string.Equals(username, i.Username, StringComparison.OrdinalIgnoreCase));

if (authenticationProvider is IHasNewUserPolicy hasNewUserPolicy)
{
Expand Down Expand Up @@ -545,7 +572,7 @@ public void AddParts(IEnumerable<IAuthenticationProvider> authenticationProvider
public void Initialize()
{
// TODO: Refactor the startup wizard so that it doesn't require a user to already exist.
var dbContext = _dbProvider.CreateContext();
using var dbContext = _dbProvider.CreateContext();

if (dbContext.Users.Any())
{
Expand Down Expand Up @@ -697,7 +724,7 @@ public void UpdatePolicy(Guid userId, UserPolicy policy)
/// <inheritdoc/>
public void ClearProfileImage(User user)
{
var dbContext = _dbProvider.CreateContext();
using var dbContext = _dbProvider.CreateContext();
dbContext.Remove(user.ProfileImage);
dbContext.SaveChanges();
}
Expand Down
3 changes: 1 addition & 2 deletions Jellyfin.Server/CoreAppHost.cs
View file
Open in desktop
Expand Up @@ -66,8 +66,7 @@ protected override void RegisterServices(IServiceCollection serviceCollection)
// TODO: Set up scoping and use AddDbContextPool
serviceCollection.AddDbContext<JellyfinDb>(
options => options
.UseSqlite($"Filename={Path.Combine(ApplicationPaths.DataPath, "jellyfin.db")}")
.UseLazyLoadingProxies(),
.UseSqlite($"Filename={Path.Combine(ApplicationPaths.DataPath, "jellyfin.db")}"),
ServiceLifetime.Transient);

serviceCollection.AddSingleton<JellyfinDbProvider>();
Expand Down