Skip to content

Conversation

@Showmick119
Copy link

Fixes #2708

Changes

  • Changed except Exception blocks to finally blocks in both sync and async versions of upload_file_chunked
  • Ensures file handles are always closed, even on successful uploads

Bug

Previously, file handles were only closed on exceptions, causing resource leaks on successful uploads.

Changed exception-only close() to finally block to ensure file handles
are always closed, preventing resource leaks on successful uploads.
@Showmick119 Showmick119 requested a review from a team as a code owner October 28, 2025 01:34
@karpetrosyan
Copy link
Collaborator

karpetrosyan commented Oct 28, 2025

Doesn't using with buf: instead of try/finally look better?

@Showmick119
Copy link
Author

Doesn't using with buf: instead of try/finally look better?

Yes you're correct, with buf: instead of try/finally does look better. Would you like me to make that modification in my PR?

@karpetrosyan
Copy link
Collaborator

Yes, please! That'd be great

@RobertCraigie RobertCraigie changed the title Fix file handle leak in upload_file_chunked fix(uploads): avoid file handle leak Oct 30, 2025
Copy link
Collaborator

@RobertCraigie RobertCraigie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Agree with Kar, using with would be better but this seems bad so I'd rather merge this fix for now, please file a follow on!

@RobertCraigie RobertCraigie changed the base branch from main to next October 30, 2025 09:49
@RobertCraigie RobertCraigie merged commit 4f1b691 into openai:next Oct 30, 2025
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

File handle resource leak in upload_file_chunked method

4 participants