Fix resource leaks, race conditions, and overly broad exception handling#6
Merged
Fix resource leaks, race conditions, and overly broad exception handling#6
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This MR addresses four bug fixes related to resource management, data consistency, and exception safety across the application and infrastructure layers.
Version bump:
1.2.0→1.2.1(PATCH — bug fixes only)Changes
1. ProcessPoolExecutor leak on shutdown
The module-level
ProcessPoolExecutorinPillowImageProcessorwas never shut down. Addedshutdown_executor()and call it from the FastAPI lifespan teardown handler.Files: pillow_processor.py, main.py
2. Race condition in retention sweeps
Concurrent calls to
get_expired()could fetch the same rows, causing double-delete attempts on storage files. Addeddelete_expired_batch()to theImageRepositoryport, implemented withSELECT … FOR UPDATE SKIP LOCKED+DELETEin a single transaction so concurrent sweeps never process the same rows.Files: image_repository.py, postgres_image_repository.py, cached_image_repository.py, apply_retention.py
3. Missing rollback in ProcessImageUseCase
If thumbnail storage succeeded but the final metadata save failed, the thumbnail was orphaned on disk and the image was stuck in
PROCESSINGstate. The finalsave()is now inside thetryblock, and theexcepthandler deletes the stored thumbnail before marking the image asFAILED.Files: process_image.py
4. Overly broad
except Exceptionhandlersexcept OSErrorfor storage operations.try/exceptwithasyncio.gather(return_exceptions=True)soKeyboardInterrupt/CancelledErrorpropagate naturally.Files: apply_retention.py, process_image.py, pipeline.py
Test coverage
test_process_image_cleans_up_thumbnail_on_save_failuretest_delete_expired_batch_delegates_and_invalidatestest_retention_deletes_expired,test_retention_no_expiredDiff summary