From 74d2c2addfd61a514c7ef04d9c08efd1f1bdb660 Mon Sep 17 00:00:00 2001 From: gnattu Date: Sat, 2 Nov 2024 17:15:00 +0800 Subject: [PATCH 1/7] Remove DynamicImageResponse local image after saved to metadata folder Previously, local images provided by DynamicImageResponse were never cleaned up until the server was restarted. This issue has become more severe in 10.10, as the default is now set to use the system's native temp folder, which might be a RAM backed tmpfs. This behavior could lead to resource starvation for long-running servers performing multiple library scans. Metadata plugins prefer the old behavior should do its own backup. --- MediaBrowser.Providers/Manager/ItemImageProvider.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MediaBrowser.Providers/Manager/ItemImageProvider.cs b/MediaBrowser.Providers/Manager/ItemImageProvider.cs index 9b738ce6f3..b371e10bff 100644 --- a/MediaBrowser.Providers/Manager/ItemImageProvider.cs +++ b/MediaBrowser.Providers/Manager/ItemImageProvider.cs @@ -232,6 +232,8 @@ namespace MediaBrowser.Providers.Manager var stream = AsyncFile.OpenRead(response.Path); await _providerManager.SaveImage(item, stream, mimeType, imageType, null, cancellationToken).ConfigureAwait(false); + + File.Delete(response.Path); } } From 469bf9d514e1b1a0e8285d2853ff053fc2a20490 Mon Sep 17 00:00:00 2001 From: gnattu Date: Sun, 3 Nov 2024 02:51:11 +0800 Subject: [PATCH 2/7] Move the remove source implementation into ProviderManager --- .../Providers/IProviderManager.cs | 3 ++- MediaBrowser.Providers/Manager/ItemImageProvider.cs | 6 +----- MediaBrowser.Providers/Manager/ProviderManager.cs | 12 ++++++++++-- .../Manager/ItemImageProviderTests.cs | 3 +++ 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/MediaBrowser.Controller/Providers/IProviderManager.cs b/MediaBrowser.Controller/Providers/IProviderManager.cs index 38fc5f2cca..0d3a334dfb 100644 --- a/MediaBrowser.Controller/Providers/IProviderManager.cs +++ b/MediaBrowser.Controller/Providers/IProviderManager.cs @@ -77,7 +77,8 @@ namespace MediaBrowser.Controller.Providers Task SaveImage(BaseItem item, Stream source, string mimeType, ImageType type, int? imageIndex, CancellationToken cancellationToken); /// - /// Saves the image. + /// Saves the image by giving the image path on filesystem. + /// This method will remove the image on the source path after saving it to the destination. /// /// Image to save. /// Source of image. diff --git a/MediaBrowser.Providers/Manager/ItemImageProvider.cs b/MediaBrowser.Providers/Manager/ItemImageProvider.cs index b371e10bff..64954818a5 100644 --- a/MediaBrowser.Providers/Manager/ItemImageProvider.cs +++ b/MediaBrowser.Providers/Manager/ItemImageProvider.cs @@ -229,11 +229,7 @@ namespace MediaBrowser.Providers.Manager { var mimeType = MimeTypes.GetMimeType(response.Path); - var stream = AsyncFile.OpenRead(response.Path); - - await _providerManager.SaveImage(item, stream, mimeType, imageType, null, cancellationToken).ConfigureAwait(false); - - File.Delete(response.Path); + await _providerManager.SaveImage(item, response.Path, mimeType, imageType, null, null, cancellationToken).ConfigureAwait(false); } } diff --git a/MediaBrowser.Providers/Manager/ProviderManager.cs b/MediaBrowser.Providers/Manager/ProviderManager.cs index 81a9af68be..3c65d49a86 100644 --- a/MediaBrowser.Providers/Manager/ProviderManager.cs +++ b/MediaBrowser.Providers/Manager/ProviderManager.cs @@ -251,7 +251,7 @@ namespace MediaBrowser.Providers.Manager } /// - public Task SaveImage(BaseItem item, string source, string mimeType, ImageType type, int? imageIndex, bool? saveLocallyWithMedia, CancellationToken cancellationToken) + public async Task SaveImage(BaseItem item, string source, string mimeType, ImageType type, int? imageIndex, bool? saveLocallyWithMedia, CancellationToken cancellationToken) { if (string.IsNullOrWhiteSpace(source)) { @@ -259,7 +259,15 @@ namespace MediaBrowser.Providers.Manager } var fileStream = AsyncFile.OpenRead(source); - return new ImageSaver(_configurationManager, _libraryMonitor, _fileSystem, _logger).SaveImage(item, fileStream, mimeType, type, imageIndex, saveLocallyWithMedia, cancellationToken); + await new ImageSaver(_configurationManager, _libraryMonitor, _fileSystem, _logger).SaveImage(item, fileStream, mimeType, type, imageIndex, saveLocallyWithMedia, cancellationToken); + try + { + File.Delete(source); + } + catch (IOException ex) + { + _logger.LogError(ex, "Source file {Source} not found or in use, skip removing", source); + } } /// diff --git a/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs b/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs index 0c7d2487cb..0d99e9af0e 100644 --- a/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs +++ b/tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs @@ -292,6 +292,9 @@ namespace Jellyfin.Providers.Tests.Manager providerManager.Setup(pm => pm.SaveImage(item, It.IsAny(), It.IsAny(), imageType, null, It.IsAny())) .Callback((callbackItem, _, _, callbackType, _, _) => callbackItem.SetImagePath(callbackType, 0, new FileSystemMetadata())) .Returns(Task.CompletedTask); + providerManager.Setup(pm => pm.SaveImage(item, It.IsAny(), It.IsAny(), imageType, null, null, It.IsAny())) + .Callback((callbackItem, _, _, callbackType, _, _, _) => callbackItem.SetImagePath(callbackType, 0, new FileSystemMetadata())) + .Returns(Task.CompletedTask); var itemImageProvider = GetItemImageProvider(providerManager.Object, null); var result = await itemImageProvider.RefreshImages(item, libraryOptions, new List { dynamicProvider.Object }, refreshOptions, CancellationToken.None); From 3aefbf8cf6062e7f82f58cc6110e22e42be556b5 Mon Sep 17 00:00:00 2001 From: gnattu Date: Sun, 3 Nov 2024 03:02:35 +0800 Subject: [PATCH 3/7] Don't do double remove in BaseDynamicImageProvider --- Emby.Server.Implementations/Images/BaseDynamicImageProvider.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/Emby.Server.Implementations/Images/BaseDynamicImageProvider.cs b/Emby.Server.Implementations/Images/BaseDynamicImageProvider.cs index 82db7c46b3..0a3d740ccf 100644 --- a/Emby.Server.Implementations/Images/BaseDynamicImageProvider.cs +++ b/Emby.Server.Implementations/Images/BaseDynamicImageProvider.cs @@ -122,7 +122,6 @@ namespace Emby.Server.Implementations.Images } await ProviderManager.SaveImage(item, outputPath, mimeType, imageType, null, false, cancellationToken).ConfigureAwait(false); - File.Delete(outputPath); return ItemUpdateType.ImageUpdate; } From e9ee0ef1f5848170f3de2dbde78c3b7d1f811eaa Mon Sep 17 00:00:00 2001 From: gnattu Date: Sun, 3 Nov 2024 04:11:41 +0800 Subject: [PATCH 4/7] Remove temp file even when saving failed --- .../Manager/ProviderManager.cs | 32 ++++++++++++++++--- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/MediaBrowser.Providers/Manager/ProviderManager.cs b/MediaBrowser.Providers/Manager/ProviderManager.cs index 3c65d49a86..bfc8ee3e15 100644 --- a/MediaBrowser.Providers/Manager/ProviderManager.cs +++ b/MediaBrowser.Providers/Manager/ProviderManager.cs @@ -258,15 +258,37 @@ namespace MediaBrowser.Providers.Manager throw new ArgumentNullException(nameof(source)); } - var fileStream = AsyncFile.OpenRead(source); - await new ImageSaver(_configurationManager, _libraryMonitor, _fileSystem, _logger).SaveImage(item, fileStream, mimeType, type, imageIndex, saveLocallyWithMedia, cancellationToken); + Exception? saveException = null; + try { - File.Delete(source); + var fileStream = AsyncFile.OpenRead(source); + await new ImageSaver(_configurationManager, _libraryMonitor, _fileSystem, _logger).SaveImage(item, fileStream, mimeType, type, imageIndex, saveLocallyWithMedia, cancellationToken); + } + catch (Exception ex) + { + saveException = ex; + _logger.LogError(ex, "Unable to save image {Source}", source); } - catch (IOException ex) + finally + { + try + { + File.Delete(source); + } + catch (IOException ex) + { + _logger.LogError(ex, "Source file {Source} not found or in use, skip removing", source); + } + catch (Exception ex) + { + saveException ??= ex; + } + } + + if (saveException is not null) { - _logger.LogError(ex, "Source file {Source} not found or in use, skip removing", source); + throw saveException; } } From bb30d26ffb05301dd3ac3255fab64431c3394463 Mon Sep 17 00:00:00 2001 From: gnattu Date: Sun, 3 Nov 2024 04:28:48 +0800 Subject: [PATCH 5/7] Use ExceptionDispatchInfo --- MediaBrowser.Providers/Manager/ProviderManager.cs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/MediaBrowser.Providers/Manager/ProviderManager.cs b/MediaBrowser.Providers/Manager/ProviderManager.cs index bfc8ee3e15..220436bf12 100644 --- a/MediaBrowser.Providers/Manager/ProviderManager.cs +++ b/MediaBrowser.Providers/Manager/ProviderManager.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Net; using System.Net.Http; using System.Net.Mime; +using System.Runtime.ExceptionServices; using System.Threading; using System.Threading.Tasks; using AsyncKeyedLock; @@ -258,7 +259,7 @@ namespace MediaBrowser.Providers.Manager throw new ArgumentNullException(nameof(source)); } - Exception? saveException = null; + ExceptionDispatchInfo? saveException = null; try { @@ -267,7 +268,7 @@ namespace MediaBrowser.Providers.Manager } catch (Exception ex) { - saveException = ex; + saveException = ExceptionDispatchInfo.Capture(ex); _logger.LogError(ex, "Unable to save image {Source}", source); } finally @@ -282,14 +283,11 @@ namespace MediaBrowser.Providers.Manager } catch (Exception ex) { - saveException ??= ex; + saveException ??= ExceptionDispatchInfo.Capture(ex); } } - if (saveException is not null) - { - throw saveException; - } + saveException?.Throw(); } /// From 03271c43a7f7f6c1ed9dca4703bd8596fd3e441e Mon Sep 17 00:00:00 2001 From: gnattu Date: Sun, 3 Nov 2024 16:10:17 +0800 Subject: [PATCH 6/7] Throw the exception as is --- MediaBrowser.Providers/Manager/ProviderManager.cs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/MediaBrowser.Providers/Manager/ProviderManager.cs b/MediaBrowser.Providers/Manager/ProviderManager.cs index 220436bf12..cc4a7ef12c 100644 --- a/MediaBrowser.Providers/Manager/ProviderManager.cs +++ b/MediaBrowser.Providers/Manager/ProviderManager.cs @@ -259,18 +259,11 @@ namespace MediaBrowser.Providers.Manager throw new ArgumentNullException(nameof(source)); } - ExceptionDispatchInfo? saveException = null; - try { var fileStream = AsyncFile.OpenRead(source); await new ImageSaver(_configurationManager, _libraryMonitor, _fileSystem, _logger).SaveImage(item, fileStream, mimeType, type, imageIndex, saveLocallyWithMedia, cancellationToken); } - catch (Exception ex) - { - saveException = ExceptionDispatchInfo.Capture(ex); - _logger.LogError(ex, "Unable to save image {Source}", source); - } finally { try @@ -281,13 +274,7 @@ namespace MediaBrowser.Providers.Manager { _logger.LogError(ex, "Source file {Source} not found or in use, skip removing", source); } - catch (Exception ex) - { - saveException ??= ExceptionDispatchInfo.Capture(ex); - } } - - saveException?.Throw(); } /// From 5769d5ca911d0b6fce7e765bfcc7c2f30602ba3f Mon Sep 17 00:00:00 2001 From: gnattu Date: Sun, 3 Nov 2024 23:25:11 +0800 Subject: [PATCH 7/7] Catch all exceptions for file removal --- MediaBrowser.Providers/Manager/ProviderManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MediaBrowser.Providers/Manager/ProviderManager.cs b/MediaBrowser.Providers/Manager/ProviderManager.cs index cc4a7ef12c..c5689550d4 100644 --- a/MediaBrowser.Providers/Manager/ProviderManager.cs +++ b/MediaBrowser.Providers/Manager/ProviderManager.cs @@ -270,7 +270,7 @@ namespace MediaBrowser.Providers.Manager { File.Delete(source); } - catch (IOException ex) + catch (Exception ex) { _logger.LogError(ex, "Source file {Source} not found or in use, skip removing", source); }