-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Core: Explicitly close SeekableInput in the AvroIterable #14322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. +1 to add a test if possible. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and +1 for adding a test
b282c4e
to
f01016a
Compare
I added a unit test and confirmed that it fails without this fix. |
f01016a
to
9c049e8
Compare
core/src/test/java/org/apache/iceberg/avro/TestAvroIterable.java
Outdated
Show resolved
Hide resolved
try (MockedStatic<AvroIO> avroIo = mockStatic(AvroIO.class)) { | ||
avroIo.when(() -> AvroIO.stream(seekableInputStream, 100L)).thenReturn(seekableInput); | ||
|
||
try (MockedStatic<DataFileReader> dataFileReaderMock = mockStatic(DataFileReader.class)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could move this into the upper try block and safe us one level of nesting
When avro file is malformed, DataFileReader.openReader can exit by throwing InvalidAvroMagicException which will leave input stream open and in the result, leaked. Co-Authored-By: Eduard Tudenhoefner <[email protected]> Co-Authored-By: Mateusz "Serafin" Gajewski <[email protected]>
146bf8c
to
3c7ee09
Compare
Thanks for the fix @ebyhr @wendigo ! Thank you for the reviews @huaxingao and @nastra ! |
When avro file is malformed, DataFileReader.openReader can exit by throwing InvalidAvroMagicException which will leave input stream open and in the result, leaked.
Supersedes #13409