Skip to content

GH-54689: [GLib] Keep the table alive in GArrowTableBatchReader - #55387

Merged
kou merged 1 commit into
apache:mainfrom
kou:glib-table-batch-reader-own-table
Nov 6, 2026
Merged

kou merged 1 commit into
apache:mainfrom
kou:glib-table-batch-reader-own-table

Conversation

@kou

@kou kou commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

Rationale for this change

If we use *arrow_table, TableBatchReader(const Table& table) is used instead of TableBatchReader(std::shared_ptr<Table> table). The former doesn't own the table. So the table may be freed while the TableBatchReader is still alive.

For example, an exported stream keeps the TableBatchReader alive after the GArrowTableBatchReader is freed.

What changes are included in this PR?

Use std::shared_ptr<Table> instead of const Table&.

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes.

Was AI used for this PR?

In accordance to the AI generation guidelines, please disclose below whether and how AI was used in this PR.

PR code and description written by:

  • Human
  • AI

Reviewed before submission by:

  • Human
  • AI
  • Not reviewed

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings November 4, 2026 07:49
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

⚠️ GitHub issue #54689 has been automatically assigned in GitHub to PR creator.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues remain for the table-reader lifetime fix.

No open issues 🎉

What changed in this PR

Fixes the table-lifetime issue behind #54689, where importing an exported table-reader stream could crash after garbage collection.

Changes:

  • Uses the owning TableBatchReader constructor so the underlying table stays alive with the reader.
  • Adds a regression test that imports and reads an exported stream after releasing the table reference and running garbage collection.
File Description
c_glib/​test/​test-table-batch-reader.rb Adds garbage-collection regression coverage with a GI bindings version guard.
c_glib/​arrow-glib/​reader.cpp Passes the table’s shared pointer to retain ownership in the C++ reader.

Review effort: Balanced

@kou

kou commented Nov 4, 2026

Copy link
Copy Markdown
Member Author

@himotoyoshi Could you try this?

@himotoyoshi

Copy link
Copy Markdown

Thanks! With this applied to 26.0.0 together with #54883, #55329 and #55333, the reproducer in #54689 no longer crashes. read_all returns the table with [1, 2, 3]. Without this PR it still segfaults.

c_glib and red-arrow ext rebuilt against Homebrew’s Arrow C++ 26.0.0; macOS 15.4.1 arm64, Ruby 3.4.1

@kou

kou commented Nov 6, 2026

Copy link
Copy Markdown
Member Author

Thanks. I'll merge this.

@kou
kou merged commit 56aefb2 into apache:main Nov 6, 2026
36 checks passed
@kou
kou deleted the glib-table-batch-reader-own-table branch November 6, 2026 01:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants