You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "fallintoplace (via GitHub)" <gi...@apache.org> on 2026/07/29 21:58:09 UTC
[PR] fix(arrow/array): always release JSON reader builder [arrow-go]
fallintoplace opened a new pull request, #1065:
URL: https://github.com/apache/arrow-go/pull/1065
## What changed
Release the JSON reader record builder whenever the final reader reference is released, independently of whether a current record batch exists.
## Why
The builder release was nested under the current-record check. This leaked the builder when the reader was released before its first read, after consuming empty input, or after reaching EOF and clearing the final record.
The regression test covers all three lifecycle paths with a checked allocator.
## Testing
- `go test ./arrow/array`
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(arrow/array): always release JSON reader builder [arrow-go]
Posted by "serramatutu (via GitHub)" <gi...@apache.org>.
serramatutu commented on code in PR #1065:
URL: https://github.com/apache/arrow-go/pull/1065#discussion_r3682694579
##########
arrow/array/json_reader_test.go:
##########
@@ -95,6 +95,38 @@ func TestJSONReaderAll(t *testing.T) {
assert.False(t, rdr.Next())
}
+func TestJSONReaderReleaseBuilder(t *testing.T) {
Review Comment:
I reverted your change to `json_reader.go` and ran the test you introduced, and the test you introduced did not break, i.e it does not reproduce any memory leak.
<img width="1073" height="1039" alt="image" src="https://github.com/user-attachments/assets/4d7d78d0-df44-486a-a565-34a1f6984a2d" />
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(arrow/array): always release JSON reader builder [arrow-go]
Posted by "serramatutu (via GitHub)" <gi...@apache.org>.
serramatutu commented on code in PR #1065:
URL: https://github.com/apache/arrow-go/pull/1065#discussion_r3682694579
##########
arrow/array/json_reader_test.go:
##########
@@ -95,6 +95,38 @@ func TestJSONReaderAll(t *testing.T) {
assert.False(t, rdr.Next())
}
+func TestJSONReaderReleaseBuilder(t *testing.T) {
Review Comment:
I reverted your change to `json_reader.go` and ran the test you introduced. The test did not break, i.e it does not reproduce any memory leak.
<img width="1073" height="1039" alt="image" src="https://github.com/user-attachments/assets/4d7d78d0-df44-486a-a565-34a1f6984a2d" />
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(arrow/array): always release JSON reader builder [arrow-go]
Posted by "serramatutu (via GitHub)" <gi...@apache.org>.
serramatutu commented on code in PR #1065:
URL: https://github.com/apache/arrow-go/pull/1065#discussion_r3682694579
##########
arrow/array/json_reader_test.go:
##########
@@ -95,6 +95,38 @@ func TestJSONReaderAll(t *testing.T) {
assert.False(t, rdr.Next())
}
+func TestJSONReaderReleaseBuilder(t *testing.T) {
Review Comment:
I reverted your change to `json_reader.go` and ran the test you introduced, and the test you introduced did not break, i.e it does not reproduce any memory leak.
<img width="1073" height="865" alt="image" src="https://github.com/user-attachments/assets/9182f1ca-ae42-4a93-baf6-0371a7f5bab9" />
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(arrow/array): always release JSON reader builder [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #1065:
URL: https://github.com/apache/arrow-go/pull/1065
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(arrow/array): always release JSON reader builder [arrow-go]
Posted by "serramatutu (via GitHub)" <gi...@apache.org>.
serramatutu commented on code in PR #1065:
URL: https://github.com/apache/arrow-go/pull/1065#discussion_r3682694579
##########
arrow/array/json_reader_test.go:
##########
@@ -95,6 +95,38 @@ func TestJSONReaderAll(t *testing.T) {
assert.False(t, rdr.Next())
}
+func TestJSONReaderReleaseBuilder(t *testing.T) {
Review Comment:
I reverted your change to `json_reader.go` and ran the test you introduced. The test did not break, i.e it does not reproduce any memory leak.
<img width="1073" height="1218" alt="Image" src="https://github.com/user-attachments/assets/d79d6274-5cfc-4d8b-9513-d2250243f385" />
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org