Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.4k
Comments
Only close file handle in ImagePalette.save() if it was opened internally#9444
Only close file handle in ImagePalette.save() if it was opened internally#9444bysiber wants to merge 4 commits intopython-pillow:mainfrom
Conversation
radarhere commentedFeb 20, 2026
Hi. You've opened 63 pull requests today after minimal prior GitHub activity. Are you an AI agent? |
bysiber commentedFeb 20, 2026
Hey, yeah I went on a bit of a contribution binge — I've been going through a bunch of popular Python libraries as a learning exercise and decided to upstream the fixes I found along the way. I realize the volume probably looks a bit unusual, sorry about that. Happy to take it slower if the review load is too much for you guys. |
radarhere commentedFeb 20, 2026 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
The review load is fine. I've createdbysiber#2 with some suggestions.
Just for the record, could you give an example of when a write would fail? |
bysiber commentedFeb 20, 2026 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Thanks for the suggestions PR, I'll merge those in. Regarding when a write could fail honestly the most realistic case is disk full (OSError), but it could also happen with network-mounted filesystems dropping mid-write, or permission quirks after the open succeeds. It's more of a defensive pattern than something you'd hit often in practice. If you think the try/finally is overkill and a plain close is fine, I'm happy to simplify it, the main fix is really just the conditional close. |
Rename variable to open_fp
ImagePalette.save()unconditionally callsfp.close()at the end, even when the file handle was passed in by the caller. This closes the caller's file object, causing any subsequent operations on it to fail withValueError: I/O operation on closed file.When
fpis a string (filename),save()correctly opens the file internally — but only that internally-opened handle should be closed. The fix tracks whether we opened the file and only closes it in that case, wrapped intry/finallyto avoid leaking the handle if a write fails.