Skip to content

Commit 68fb464

Browse files
authored
GH-50009: [R] FinalizeS3 segfaults for stale connection (#50081)
### Rationale for this change User experiences issues with process crashing when reading/writing from S3. Looks like a stale connection and sigpipe stuff. See also #32026 ### What changes are included in this PR? Install sigpipe handler upon S3 initialisation so it'll not kill the process. ### Are these changes tested? No - and I'm not sure how I can really test this out. ### Are there any user-facing changes? No * GitHub Issue: #50009 Authored-by: Nic Crane <thisisnic@gmail.com> Signed-off-by: Nic Crane <thisisnic@gmail.com>
1 parent e980b7e commit 68fb464

1 file changed

Lines changed: 28 additions & 3 deletions

File tree

r/src/filesystem.cpp

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,10 +256,23 @@ std::string fs___SubTreeFileSystem__base_path(
256256
return file_system->base_path();
257257
}
258258

259+
// Forward declaration - defined in the ARROW_R_WITH_S3 block below.
260+
#if defined(ARROW_R_WITH_S3)
261+
void EnsureS3InitializedWithSigpipeHandler();
262+
#endif
263+
259264
// [[arrow::export]]
260265
cpp11::writable::list fs___FileSystemFromUri(const std::string& path) {
261266
using cpp11::literals::operator""_nm;
262267

268+
#if defined(ARROW_R_WITH_S3)
269+
// Initialize S3 before FileSystemFromUri so our options (with SIGPIPE handler)
270+
// take effect before the C++ library's internal EnsureS3Initialized() call.
271+
if (path.substr(0, 5) == "s3://") {
272+
EnsureS3InitializedWithSigpipeHandler();
273+
}
274+
#endif
275+
263276
std::string out_path;
264277
auto io_context = MainRThread::GetInstance().CancellableIOContext();
265278
return cpp11::writable::list({"fs"_nm = cpp11::to_r6(ValueOrStop(
@@ -281,6 +294,20 @@ void fs___CopyFiles(const std::shared_ptr<fs::FileSystem>& source_fs,
281294

282295
#include <arrow/filesystem/s3fs.h>
283296

297+
// Initialize S3 with the SIGPIPE handler enabled. Without it, stale connections
298+
// in the SDK's connection pool can trigger SIGPIPE during Aws::ShutdownAPI(),
299+
// which causes R's signal handler to longjmp out of the teardown and segfault
300+
// (GH-50009, GH-32026).
301+
void EnsureS3InitializedWithSigpipeHandler() {
302+
fs::S3GlobalOptions options = fs::S3GlobalOptions::Defaults();
303+
options.install_sigpipe_handler = true;
304+
auto status = fs::InitializeS3(options);
305+
// InitializeS3 returns Invalid if already initialized - that's fine
306+
if (!status.ok() && !fs::IsS3Initialized()) {
307+
StopIfNotOk(status);
308+
}
309+
}
310+
284311
// [[s3::export]]
285312
std::shared_ptr<fs::S3FileSystem> fs___S3FileSystem__create(
286313
bool anonymous = false, std::string access_key = "", std::string secret_key = "",
@@ -291,9 +318,7 @@ std::shared_ptr<fs::S3FileSystem> fs___S3FileSystem__create(
291318
bool allow_bucket_creation = false, bool allow_bucket_deletion = false,
292319
bool check_directory_existence_before_creation = false, double connect_timeout = -1,
293320
double request_timeout = -1) {
294-
// We need to ensure that S3 is initialized before we start messing with the
295-
// options
296-
StopIfNotOk(fs::EnsureS3Initialized());
321+
EnsureS3InitializedWithSigpipeHandler();
297322
fs::S3Options s3_opts;
298323
// Handle auth (anonymous, keys, default)
299324
// (validation/internal coherence handled in R)

0 commit comments

Comments
 (0)