Fix various things that might keep thumbnail files locked (BL-16122)#7808
Fix various things that might keep thumbnail files locked (BL-16122)#7808JohnThomson wants to merge 1 commit intomasterfrom
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚩 RebuildThumbNail has an additional undisposed image path for readonly thumbnails
In RebuildThumbNail at BookThumbNailer.cs:536-543, when the thumbnail is readonly, TryGetPremadeThumbnail returns an image loaded from file (caller-owned per the new comments). This image is passed to callback(book.BookInfo, thumb). For RebuildThumbNailNow (line 501), the callback disposes it correctly. For CollectionModel.UpdateThumbnailAsync (line 724-728), it also disposes correctly. However, for the 4-parameter UpdateThumbnailAsync overload at line 706, callers provide their own callbacks and may not dispose the image — this is a pre-existing concern not introduced by this PR, but worth noting since the PR adds ownership documentation.
(Refers to lines 536-543)
Was this helpful? React with 👍 or 👎 to provide feedback.
| book, | ||
| thumbnailOptions, | ||
| (info, image) => { }, | ||
| (info, image) => image?.Dispose(), |
There was a problem hiding this comment.
🔴 Disposing shared Resources.Error70x70 singleton in RebuildThumbNailNow callback
The new callback (info, image) => image?.Dispose() will dispose Resources.Error70x70 when error paths are hit. In GetThumbNailOfBookCover (BookThumbNailer.cs:59,66,100,106), the callback receives Resources.Error70x70 directly (not a clone). This flows through RebuildThumbNail at BookThumbNailer.cs:551 (image => callback(book.BookInfo, image)) and reaches the disposing callback. Since ResourceManager.GetObject caches and returns the same Bitmap instance on each access, disposing it corrupts the shared resource. All subsequent uses of Resources.Error70x70 elsewhere (e.g., CollectionModel.cs:746 calling .Save(), CollectionApi.cs:775 calling ConvertImageToStream()) will fail with ObjectDisposedException or GDI+ errors.
Prompt for agents
The callback in RebuildThumbNailNow disposes the image it receives, but GetThumbNailOfBookCover (lines 59, 66, 100, 106) passes the shared Resources.Error70x70 singleton directly to the callback in several error paths. Disposing this shared resource corrupts it for all future uses across the application.
Possible fixes:
1. In RebuildThumbNailNow, only dispose images that are not Resources.Error70x70 (fragile, not recommended).
2. In GetThumbNailOfBookCover, clone Resources.Error70x70 before passing it to callbacks, so callers can safely dispose the clone. This would be consistent with CreateThumbnailOfCoverImage (line 319) which already clones its image before calling the callback.
3. Have the callback check whether the image is a shared resource before disposing it.
Option 2 is likely the cleanest: change all callback(Resources.Error70x70) calls in GetThumbNailOfBookCover to callback(Resources.Error70x70.Clone() as Image), matching the pattern already used in CreateThumbnailOfCoverImage at line 319.
Was this helpful? React with 👍 or 👎 to provide feedback.
| (bookInfo, image) => | ||
| { | ||
| RefreshOneThumbnail(bookInfo, image); | ||
| image?.Dispose(); | ||
| }, |
There was a problem hiding this comment.
🔴 Disposing shared Resources.Error70x70 singleton in UpdateThumbnailAsync callback
The new callback wrapping RefreshOneThumbnail calls image?.Dispose() on the image it receives. Through the same error paths in GetThumbNailOfBookCover (BookThumbNailer.cs:66,100,106), the image can be the shared Resources.Error70x70 singleton. The flow is: UpdateThumbnailAsync → RebuildThumbNailAsync → RebuildThumbNail → GetThumbNailOfBookCover → error path passes Resources.Error70x70 to callback → disposed. Note that while ErrorBook is filtered at CollectionModel.cs:713, the HasFatalError, CreateThumbnailOfCoverImage failure, and exception-catch paths in GetThumbNailOfBookCover are still reachable. This is the same root cause as the RebuildThumbNailNow bug but in a different call site.
Prompt for agents
The callback added in UpdateThumbnailAsync(Book.Book book) disposes the image, but error paths in GetThumbNailOfBookCover (BookThumbNailer.cs:66,100,106) pass the shared Resources.Error70x70 singleton to the callback. Disposing it corrupts the shared resource.
The fix should be coordinated with the same issue in RebuildThumbNailNow. The cleanest approach is to ensure GetThumbNailOfBookCover always passes a clone of Resources.Error70x70 (matching the pattern in CreateThumbnailOfCoverImage at line 319 which already clones). This way all callers can safely dispose the image they receive.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
This change is