Skip to content

GH-50930: Prevent symbol interposition in arrow_s3fs - #50949

Open
tadeja wants to merge 1 commit into
apache:mainfrom
tadeja:50930-initialize-S3
Open

GH-50930: Prevent symbol interposition in arrow_s3fs#50949
tadeja wants to merge 1 commit into
apache:mainfrom
tadeja:50930-initialize-S3

Conversation

@tadeja

@tadeja tadeja commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Rationale for this change

Fix #50930 Actual: "Invalid: Attempt to initialize S3 after it has been finalized"
[ FAILED ] S3Test.FromUriRejectsUnknownOptions (0 ms)

Both libarrow.so and dynamically loaded libarrow_s3fs.so compile s3fs.cc, so both libraries get own S3 state and filesystem registration callbacks.
The S3 module tests finalize the state in libarrow.so before loading libarrow_s3fs.so. ELF symbol interposition causes libarrow_sf3s.so references to bind to definitions in libarrow.so, so the module incorrectly reuses its finalized state.
LD_DEBUG=bindings example output (for symbol arrow::fs::kS3FileSystemModule):

12833:	binding file /build/cpp/debug/libarrow_s3fs.so [0] to /build/cpp/debug/libarrow.so.2600 [0]: normal symbol `_ZN5arrow2fs19kS3FileSystemModuleE'

These duplicate definitions were present before, but the issue was masked by -fvisibility-inlines-hidden by conda-forge compilers 1.11 (with gcc 14.4).
Now with conda-forge compilers 2.0 there is no -fvisibility-inlines-hidden anymore, minimally activated as described in conda-forge/conda-forge.github.io#2595.

What changes are included in this PR?

Link arrow_s3fs with -Wl,-Bsymbolic-functions so its function references bind locally,
and make kS3FileSystemModule static so it's not exported.

Are these changes tested?

Yes, S3Test.FromUriRejectsUnknownOptions passes.

Are there any user-facing changes?

No.

@github-actions

Copy link
Copy Markdown

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

@tadeja tadeja changed the title GH-50930: Link arrow_s3fs with -Bsymbolic-functions GH-50930: Prevent symbol interposition in arrow_s3fs Aug 24, 2026
@tadeja
tadeja marked this pull request as ready for review August 24, 2026 19:30
@tadeja
tadeja requested a review from pitrou as a code owner August 24, 2026 19:30
Copilot AI lite review requested due to automatic review settings August 24, 2026 19:30

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.

Pull request overview

Fixes ELF symbol interposition that could cause arrow_s3fs to reuse finalized S3 state from libarrow.

Changes:

  • Makes the S3 filesystem registrar local.
  • Detects linker support and applies -Bsymbolic-functions to arrow_s3fs.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
cpp/src/arrow/filesystem/s3fs.cc Localizes the S3 registrar.
cpp/src/arrow/CMakeLists.txt Applies symbolic function binding to the S3 module.
cpp/CMakeLists.txt Detects linker support for the required option.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@pitrou

pitrou commented Aug 25, 2026

Copy link
Copy Markdown
Member

Both libarrow.so and dynamically loaded libarrow_s3fs.so compile s3fs.cc

Isn't that the actual problem? I don't think we should compile the same file twice. @raulcd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[C++][CI] S3 tests fail with "Attempt to initialize S3 after it has been finalized"

3 participants