You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/05/30 03:47:51 UTC
[GitHub] [arrow] emkornfield opened a new pull request #10423: ARROW-12907: [Java] Fix memory leak on deserialization errors
emkornfield opened a new pull request #10423:
URL: https://github.com/apache/arrow/pull/10423
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] liyafan82 commented on pull request #10423: ARROW-12907: [Java] Fix memory leak on deserialization errors
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on pull request #10423:
URL: https://github.com/apache/arrow/pull/10423#issuecomment-850985161
> CC @liyafan82 or @lidavidm would you mind reviewing?
@emkornfield Thank you for working on this. I will take a look two or three days later, when I come back from vocation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on pull request #10423: ARROW-12907: [Java] Fix memory leak on deserialization errors
Posted by GitBox <gi...@apache.org>.
emkornfield commented on pull request #10423:
URL: https://github.com/apache/arrow/pull/10423#issuecomment-850935302
In practice this can happen if the thread running this code is interrupted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kiszk commented on pull request #10423: ARROW-12907: [Java] Fix memory leak on deserialization errors
Posted by GitBox <gi...@apache.org>.
kiszk commented on pull request #10423:
URL: https://github.com/apache/arrow/pull/10423#issuecomment-851040065
Good catch, the fix looks good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] liyafan82 closed pull request #10423: ARROW-12907: [Java] Fix memory leak on deserialization errors
Posted by GitBox <gi...@apache.org>.
liyafan82 closed pull request #10423:
URL: https://github.com/apache/arrow/pull/10423
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] liyafan82 commented on pull request #10423: ARROW-12907: [Java] Fix memory leak on deserialization errors
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on pull request #10423:
URL: https://github.com/apache/arrow/pull/10423#issuecomment-854300731
Merging. Thanks for the PR. @emkornfield
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] liyafan82 commented on a change in pull request #10423: ARROW-12907: [Java] Fix memory leak on deserialization errors
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on a change in pull request #10423:
URL: https://github.com/apache/arrow/pull/10423#discussion_r643604065
##########
File path: java/vector/src/main/java/org/apache/arrow/vector/ipc/message/MessageSerializer.java
##########
@@ -723,8 +723,13 @@ public static MessageMetadataResult readMessage(ReadChannel in) throws IOExcepti
public static ArrowBuf readMessageBody(ReadChannel in, long bodyLength,
BufferAllocator allocator) throws IOException {
ArrowBuf bodyBuffer = allocator.buffer(bodyLength);
Review comment:
Nice catch!
A more conventional way is to wrap the statements in try-with-resource block?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #10423: ARROW-12907: [Java] Fix memory leak on deserialization errors
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10423:
URL: https://github.com/apache/arrow/pull/10423#issuecomment-850934931
https://issues.apache.org/jira/browse/ARROW-12907
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] liyafan82 closed pull request #10423: ARROW-12907: [Java] Fix memory leak on deserialization errors
Posted by GitBox <gi...@apache.org>.
liyafan82 closed pull request #10423:
URL: https://github.com/apache/arrow/pull/10423
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] liyafan82 commented on a change in pull request #10423: ARROW-12907: [Java] Fix memory leak on deserialization errors
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on a change in pull request #10423:
URL: https://github.com/apache/arrow/pull/10423#discussion_r643606492
##########
File path: java/vector/src/test/java/org/apache/arrow/vector/ipc/MessageSerializerTest.java
##########
@@ -197,12 +199,30 @@ public void testSerializeRecordBatchV5() throws IOException {
IpcOption option = new IpcOption(false, MetadataVersion.V5);
ByteArrayOutputStream out = new ByteArrayOutputStream();
MessageSerializer.serialize(new WriteChannel(Channels.newChannel(out)), batch, option);
+ validityb.close();
+ valuesb.close();
+ batch.close();
+
+ {
+ ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray());
Review comment:
It would be nice to wrap this into a try-with-resource block.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] liyafan82 commented on pull request #10423: ARROW-12907: [Java] Fix memory leak on deserialization errors
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on pull request #10423:
URL: https://github.com/apache/arrow/pull/10423#issuecomment-854300731
Merging. Thanks for the PR. @emkornfield
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on pull request #10423: ARROW-12907: [Java] Fix memory leak on deserialization errors
Posted by GitBox <gi...@apache.org>.
emkornfield commented on pull request #10423:
URL: https://github.com/apache/arrow/pull/10423#issuecomment-850934930
CC @liyafan82 or @lidavidm would you mind reviewing?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org