diff --git a/src/BloomExe/Book/BookInfo.cs b/src/BloomExe/Book/BookInfo.cs
index a629d69e3051..8bd252a7931d 100644
--- a/src/BloomExe/Book/BookInfo.cs
+++ b/src/BloomExe/Book/BookInfo.cs
@@ -355,6 +355,7 @@ public bool TryGetPremadeThumbnail(out Image image)
{
try
{
+ // Caller owns this image and must dispose it promptly to release any file lock.
image = RobustImageIO.GetImageFromFile(path);
return true;
}
diff --git a/src/BloomExe/Book/BookStarter.cs b/src/BloomExe/Book/BookStarter.cs
index c01f2118489f..dc0b0f83044f 100644
--- a/src/BloomExe/Book/BookStarter.cs
+++ b/src/BloomExe/Book/BookStarter.cs
@@ -71,7 +71,7 @@ string parentCollectionPath
var newBookFolder = Path.Combine(parentCollectionPath, initialBookName);
CopyFolder(sourceBookFolder, newBookFolder);
BookStorage.RemoveLocalOnlyFiles(newBookFolder);
- //if something bad happens from here on out, we need to delete that folder we just made
+ //if something bad happens from here on out, we need to delete that folder we just made
try
{
var oldNamedFile = Path.Combine(
@@ -93,7 +93,11 @@ string parentCollectionPath
RobustFile.Move(oldNamedFile, newNamedFile);
//the destination may change here...
- newBookFolder = SetupNewDocumentContents(sourceBookFolder, newBookFolder, newBookInstanceId);
+ newBookFolder = SetupNewDocumentContents(
+ sourceBookFolder,
+ newBookFolder,
+ newBookInstanceId
+ );
if (OnNextRunSimulateFailureMakingBook)
throw new ApplicationException("Simulated failure for unit test");
@@ -166,7 +170,11 @@ private string GetMetaValue(SafeXmlDocument Dom, string name, string defaultValu
return defaultValue;
}
- private string SetupNewDocumentContents(string sourceFolderPath, string initialPath, string newBookInstanceId)
+ private string SetupNewDocumentContents(
+ string sourceFolderPath,
+ string initialPath,
+ string newBookInstanceId
+ )
{
// This bookInfo is temporary, just used to make the (also temporary) BookStorage we
// use here in this method. I don't think it actually matters what its save context is.
@@ -416,7 +424,11 @@ private static void ProcessXMatterMetaTags(BookStorage storage)
storage.Dom.RemoveMetaElement("xmatter-for-children");
}
- private void SetLineageAndId(BookStorage storage, string sourceFolderPath, string newBookInstanceId)
+ private void SetLineageAndId(
+ BookStorage storage,
+ string sourceFolderPath,
+ string newBookInstanceId
+ )
{
string parentId = null;
string lineage = null;
diff --git a/src/BloomExe/Book/BookStorage.cs b/src/BloomExe/Book/BookStorage.cs
index 3bedaad7c6e6..43ebb4661bba 100644
--- a/src/BloomExe/Book/BookStorage.cs
+++ b/src/BloomExe/Book/BookStorage.cs
@@ -45,6 +45,8 @@ public interface IBookStorage
string FolderName { get; }
string FolderPath { get; }
string PathToExistingHtml { get; }
+
+ // Caller owns the returned image and must dispose it promptly.
bool TryGetPremadeThumbnail(string fileName, out Image image);
//bool DeleteBook();
@@ -435,6 +437,7 @@ public bool TryGetPremadeThumbnail(string fileName, out Image image)
string path = Path.Combine(FolderPath, fileName);
if (RobustFile.Exists(path))
{
+ // Caller owns this image and must dispose it promptly to release any file lock.
image = RobustImageIO.GetImageFromFile(path);
return true;
}
diff --git a/src/BloomExe/BookThumbNailer.cs b/src/BloomExe/BookThumbNailer.cs
index c5c4c8a14e27..374c752fb2bc 100644
--- a/src/BloomExe/BookThumbNailer.cs
+++ b/src/BloomExe/BookThumbNailer.cs
@@ -46,6 +46,11 @@ public HtmlThumbNailer HtmlThumbNailer
get { return _thumbnailProvider; }
}
+ private static Image CreateDisposableErrorThumbnail()
+ {
+ return Resources.Error70x70.Clone() as Image;
+ }
+
private void GetThumbNailOfBookCover(
Book.Book book,
HtmlThumbNailer.ThumbnailOptions thumbnailOptions,
@@ -56,14 +61,14 @@ bool async
{
if (book is ErrorBook)
{
- callback(Resources.Error70x70);
+ callback(CreateDisposableErrorThumbnail());
return;
}
try
{
if (book.HasFatalError) //NB: we might not know yet... we don't fully load every book just to show its thumbnail
{
- callback(Resources.Error70x70);
+ callback(CreateDisposableErrorThumbnail());
return;
}
GenerateImageForWeb(book);
@@ -79,7 +84,7 @@ bool async
var dom = book.GetPreviewXmlDocumentForFirstPage();
if (dom == null)
{
- callback(Resources.Error70x70);
+ callback(CreateDisposableErrorThumbnail());
return;
}
string folderForCachingThumbnail;
@@ -97,13 +102,13 @@ bool async
#else
if (!CreateThumbnailOfCoverImage(book, thumbnailOptions, callback))
{
- callback(Resources.Error70x70);
+ callback(CreateDisposableErrorThumbnail());
}
#endif
}
catch (Exception err)
{
- callback(Resources.Error70x70);
+ callback(CreateDisposableErrorThumbnail());
errorCallback(err);
Debug.Fail(err.Message);
}
@@ -469,7 +474,8 @@ private void SetSizeAndOrientationClass(SafeXmlElement pageDiv, string sizeOrien
}
///
- /// Will call either 'callback' or 'errorCallback' UNLESS the thumbnail is readonly, in which case it will do neither.
+ /// Will call either 'callback' or 'errorCallback'.
+ /// The callback receives an image that the callback must dispose when done.
///
///
///
@@ -498,7 +504,7 @@ HtmlThumbNailer.ThumbnailOptions thumbnailOptions
RebuildThumbNail(
book,
thumbnailOptions,
- (info, image) => { },
+ (info, image) => image?.Dispose(),
(info, ex) =>
{
throw ex;
@@ -517,7 +523,8 @@ Book.Book book
}
///
- /// Will call either 'callback' or 'errorCallback' UNLESS the thumbnail is readonly, in which case it will do neither.
+ /// Will call either 'callback' or 'errorCallback'.
+ /// The callback receives an image that the callback must dispose when done.
///
///
///
diff --git a/src/BloomExe/CollectionTab/CollectionModel.cs b/src/BloomExe/CollectionTab/CollectionModel.cs
index a925357ac2ae..1b5f6212080a 100644
--- a/src/BloomExe/CollectionTab/CollectionModel.cs
+++ b/src/BloomExe/CollectionTab/CollectionModel.cs
@@ -721,7 +721,17 @@ public void UpdateThumbnailAsync(Book.Book book)
UpdateThumbnailAsync(
book,
BookThumbNailer.GetCoverThumbnailOptions(-1, Guid.Empty),
- RefreshOneThumbnail,
+ (bookInfo, image) =>
+ {
+ try
+ {
+ RefreshOneThumbnail(bookInfo, image);
+ }
+ finally
+ {
+ image?.Dispose();
+ }
+ },
HandleThumbnailerErrror
);
}
diff --git a/src/BloomExe/ImageProcessing/RuntimeImageProcessor.cs b/src/BloomExe/ImageProcessing/RuntimeImageProcessor.cs
index 1cc27cd5bddb..658ad9e1e1cd 100644
--- a/src/BloomExe/ImageProcessing/RuntimeImageProcessor.cs
+++ b/src/BloomExe/ImageProcessing/RuntimeImageProcessor.cs
@@ -225,36 +225,37 @@ Color backColor
(newW * originalImage.Image.Height + (originalImage.Image.Width / 2))
/ originalImage.Image.Width;
- var thumbnail = new Bitmap(newW, newH);
-
- var g = Graphics.FromImage(thumbnail);
- if (backColor != Color.Empty)
+ using (var thumbnail = new Bitmap(newW, newH))
+ using (var g = Graphics.FromImage(thumbnail))
{
- using (var brush = new SolidBrush(backColor))
+ if (backColor != Color.Empty)
{
- g.FillRectangle(brush, new Rectangle(0, 0, newW, newH));
+ using (var brush = new SolidBrush(backColor))
+ {
+ g.FillRectangle(brush, new Rectangle(0, 0, newW, newH));
+ }
}
+ Image imageToDraw = originalImage.Image;
+ bool makeTransparentImage = ImageUtils.ShouldMakeBackgroundTransparent(
+ originalImage
+ );
+ if (makeTransparentImage)
+ {
+ imageToDraw = MakePngBackgroundTransparent(originalImage);
+ }
+ var destRect = new Rectangle(0, 0, newW, newH);
+ // Note the image size may change when the background is made transparent.
+ // See https://silbloom.myjetbrains.com/youtrack/issue/BL-5632.
+ g.DrawImage(
+ imageToDraw,
+ destRect,
+ new Rectangle(0, 0, imageToDraw.Width, imageToDraw.Height),
+ GraphicsUnit.Pixel
+ );
+ if (makeTransparentImage)
+ imageToDraw.Dispose();
+ RobustImageIO.SaveImage(thumbnail, pathToProcessedImage);
}
- Image imageToDraw = originalImage.Image;
- bool makeTransparentImage = ImageUtils.ShouldMakeBackgroundTransparent(
- originalImage
- );
- if (makeTransparentImage)
- {
- imageToDraw = MakePngBackgroundTransparent(originalImage);
- }
- var destRect = new Rectangle(0, 0, newW, newH);
- // Note the image size may change when the background is made transparent.
- // See https://silbloom.myjetbrains.com/youtrack/issue/BL-5632.
- g.DrawImage(
- imageToDraw,
- destRect,
- new Rectangle(0, 0, imageToDraw.Width, imageToDraw.Height),
- GraphicsUnit.Pixel
- );
- if (makeTransparentImage)
- imageToDraw.Dispose();
- RobustImageIO.SaveImage(thumbnail, pathToProcessedImage);
}
return true;
diff --git a/src/BloomExe/web/RequestInfo.cs b/src/BloomExe/web/RequestInfo.cs
index 12887525b3c0..d576837c12a2 100644
--- a/src/BloomExe/web/RequestInfo.cs
+++ b/src/BloomExe/web/RequestInfo.cs
@@ -13,6 +13,7 @@
using Bloom.Book;
using Bloom.web;
using Bloom.web.controllers;
+using SIL.Code;
using SIL.IO;
using SIL.Reporting;
@@ -136,6 +137,23 @@ private static void ReportHttpListenerProblem(HttpListenerException e)
public bool HaveFullyProcessedRequest { get; private set; }
+ // We intentionally do not use RobustFile.OpenRead() in this endpoint.
+ // We need a read stream that permits concurrent overwrite/delete of the same file
+ // while it is being served (notably for thumbnails and media), so we require
+ // FileShare.ReadWrite | FileShare.Delete. We still want robust retry behavior,
+ // so we wrap this open in RetryUtility.Retry().
+ private static FileStream OpenSharedReadStreamWithRetry(string path)
+ {
+ return RetryUtility.Retry(() =>
+ new FileStream(
+ path,
+ FileMode.Open,
+ FileAccess.Read,
+ FileShare.ReadWrite | FileShare.Delete
+ )
+ );
+ }
+
public void ReplyWithFileContent(string path, string originalPath = null)
{
//Deal with BL-3153, where the file was still open in another thread
@@ -152,7 +170,7 @@ public void ReplyWithFileContent(string path, string originalPath = null)
try
{
- fs = RobustFile.OpenRead(path);
+ fs = OpenSharedReadStreamWithRetry(path);
}
catch (Exception error)
{
@@ -255,7 +273,7 @@ public void ReplyWithFileContent(string path, string originalPath = null)
_actualContext.Response.OutputStream.Write(buffer, 0, read);
try
{
- fs = RobustFile.OpenRead(path);
+ fs = OpenSharedReadStreamWithRetry(path);
}
catch (FileNotFoundException)
{