(arrow) branch main updated: GH-50239: [R] Data race issue from R API requests in parallel region (#50488)

[thisisnic]

This is an automated email from the ASF dual-hosted git repository.

thisisnic pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/main by this push:
     new 4da2a7e5ff GH-50239: [R] Data race issue from R API requests in 
parallel region (#50488)
4da2a7e5ff is described below

commit 4da2a7e5ff91052e71775e29d9227f3ae5a8615b
Author: Nic Crane <thisis...@gmail.com>
AuthorDate: Sun Sep 6 10:58:38 2026 +0100

    GH-50239: [R] Data race issue from R API requests in parallel region 
(#50488)
    
    ### Rationale for this change
    
    When `use_threads = TRUE`, parallel tasks are submitted to the thread pool 
immediately during `RTasks::Append()`. This means worker threads start reading 
R vector headers (via `INTEGER()`, `REAL()`, etc.) while the main thread is 
still assigning vectors into the output list via `SET_VECTOR_ELT`, which writes 
to the same header memory. ThreadSanitizer detects this as a data race.
    
    ### What changes are included in this PR?
    
    Delay submission of parallel tasks until `RTasks::Finish()`, matching the 
existing pattern for serial tasks. All R-side allocations and list assignments 
complete on the main thread before any worker threads start.
    
    ### Are these changes tested?
    
    Existing tests cover the affected code path.
    I also ran some local benchmarks to see if this change affected 
performance, but seems fine.
    
    ### Are there any user-facing changes?
    
    No.
    * GitHub Issue: #50239
    
    Authored-by: Nic Crane <thisis...@gmail.com>
    Signed-off-by: Nic Crane <thisis...@gmail.com>
---
 r/src/RTasks.cpp     | 8 ++++++--
 r/src/r_task_group.h | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/r/src/RTasks.cpp b/r/src/RTasks.cpp
index 5d3a749b6d..09c5dd630f 100644
--- a/r/src/RTasks.cpp
+++ b/r/src/RTasks.cpp
@@ -40,8 +40,11 @@ Status RTasks::Finish() {
     }
   }
 
-  // then wait for the parallel tasks to finish
+  // submit the delayed parallel tasks and wait for them to finish
   if (use_threads_) {
+    for (auto& task : delayed_parallel_tasks_) {
+      parallel_tasks_->Append(std::move(task));
+    }
     status &= parallel_tasks_->Finish();
   }
 
@@ -50,7 +53,7 @@ Status RTasks::Finish() {
 
 void RTasks::Append(bool parallel, RTasks::Task&& task) {
   if (parallel && use_threads_) {
-    parallel_tasks_->Append(std::move(task));
+    delayed_parallel_tasks_.push_back(std::move(task));
   } else {
     delayed_serial_tasks_.push_back(std::move(task));
   }
@@ -58,6 +61,7 @@ void RTasks::Append(bool parallel, RTasks::Task&& task) {
 
 void RTasks::Reset() {
   delayed_serial_tasks_.clear();
+  delayed_parallel_tasks_.clear();
 
   stop_source_.Reset();
   if (use_threads_) {
diff --git a/r/src/r_task_group.h b/r/src/r_task_group.h
index e1c298b27f..43c040b9a7 100644
--- a/r/src/r_task_group.h
+++ b/r/src/r_task_group.h
@@ -45,6 +45,7 @@ class RTasks {
   StopSource stop_source_;
   std::shared_ptr<arrow::internal::TaskGroup> parallel_tasks_;
   std::vector<Task> delayed_serial_tasks_;
+  std::vector<Task> delayed_parallel_tasks_;
 };
 
 }  // namespace r