[C++] Data race between future signalling and destruction
- Dominant language
- C++
- Stars
- 17.1k
- Forks
- 4.3k
- Avg merge
- 3d 13h
- Merged PRs (30d)
- 88
Description
This sporadic Thread Sanitizer error just occurred to me:
```Java
WARNING: ThreadSanitizer: data race (pid=636020)
Write of size 8 at 0x7b2c000017d0 by main thread:
#0 pthread_cond_destroy ../../../../libsanitizer/tsan/tsan_interceptors_posix.cpp:1208 (libtsan.so.0+0x31c14)
#1 arrow::ConcreteFutureImpl::~ConcreteFutureImpl() /home/antoine/arrow/dev/cpp/src/arrow/util/future.cc:211 (libarrow.so.900+0xa70b62)
#2 arrow::ConcreteFutureImpl::~ConcreteFutureImpl() /home/antoine/arrow/dev/cpp/src/arrow/util/future.cc:211 (libarrow.so.900+0xa70ba0)
#3 std::default_delete::operator()(arrow::FutureImpl*) const /home/antoine/miniconda3/envs/pyarrow/x86_64-conda-linux-gnu/include/c++/10.3.0/bits/unique_ptr.h:85 (arrow-dataset-file-test+0x584a1)
#4 std::_Sp_counted_deleter, std::allocator, (__gnu_cxx::_Lock_policy)2>::_M_dispose() /home/antoine/miniconda3/envs/pyarrow/x86_64-conda-linux-gnu/include/c++/10.3.0/bits/shared_ptr_base.h:474 (arrow-dataset-file-test+0xa9638)
#5 std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (libarrow.so.900+0x2e1158)
#6 std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() (libarrow.so.900+0x2dc6ed)
#7 std::__shared_ptr::~__shared_ptr() (libarrow.so.900+0x978fee)
#8 std::shared_ptr::~shared_ptr() (libarrow.so.900+0x97901c)
#9 arrow::Future::~Future() (libarrow.so.900+0x97904a)
#10 ~ExecPlanImpl /home/antoine/arrow/dev/cpp/src/arrow/compute/exec/exec_plan.cc:52 (libarrow.so.900+0xe8160b)
#11 ~ExecPlanImpl /home/antoine/arrow/dev/cpp/src/arrow/compute/exec/exec_plan.cc:58 (libarrow.so.900+0xe8166e)
#12 _M_dispose /home/antoine/miniconda3/envs/pyarrow/x86_64-conda-linux-gnu/include/c++/10.3.0/bits/shared_ptr_base.h:380 (libarrow.so.900+0xea6c2a)
#13 std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (libarrow_dataset.so.900+0x7bd10)
#14 std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() /home/antoine/miniconda3/envs/pyarrow/x86_64-conda-linux-gnu/include/c++/10.3.0/bits/shared_ptr_base.h:733 (libarrow_dataset.so.900+0x77ad9)
#15 std::__shared_ptr::~__shared_ptr() /home/antoine/miniconda3/envs/pyarrow/x86_64-conda-linux-gnu/include/c++/10.3.0/bits/shared_ptr_base.h:1183 (libarrow_dataset.so.900+0xd3dfc)
#16 std::shared_ptr::~shared_ptr() /home/antoine/miniconda3/envs/pyarrow/x86_64-conda-linux-gnu/include/c++/10.3.0/bits/shared_ptr.h:121 (libarrow_dataset.so.900+0xd3e2a)
#17 arrow::dataset::FileSystemDataset::Write(arrow::dataset::FileSystemDatasetWriteOptions const&, std::shared_ptr) /home/antoine/arrow/dev/cpp/src/arrow/dataset/file_base.cc:398 (libarrow_dataset.so.900+0xd49ca)
#18 arrow::dataset::TestFileSystemDataset_WriteProjected_Test::TestBody() /home/antoine/arrow/dev/cpp/src/arrow/dataset/file_test.cc:330 (arrow-dataset-file-test+0x2e382)
#19 void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (libgtest.so.1.11.0+0x5bd3d)
Previous read of size 8 at 0x7b2c000017d0 by thread T3:
#0 pthread_cond_broadcast ../../../../libsanitizer/tsan/tsan_interceptors_posix.cpp:1201 (libtsan.so.0+0x31b51)
#1 arrow::ConcreteFutureImpl::DoMarkFinishedOrFailed(arrow::FutureState) /home/antoine/arrow/dev/cpp/src/arrow/util/future.cc:343 (libarrow.so.900+0xa6bee0)
#2 arrow::ConcreteFutureImpl::DoMarkFinished() /home/antoine/arrow/dev/cpp/src/arrow/util/future.cc:232 (libarrow.so.900+0xa6b0f4)
#3 arrow::FutureImpl::MarkFinished() /home/antoine/arrow/dev/cpp/src/arrow/util/future.cc:409 (libarrow.so.900+0xa6c83f)
#4 arrow::Future::DoMarkFinished(arrow::Result) /home/antoine/arrow/dev/cpp/src/arrow/util/future.h:725 (libarrow.so.900+0x9cbf81)
#5 void arrow::Future::MarkFinished(arrow::Status) /home/antoine/arrow/dev/cpp/src/arrow/util/future.h:476 (libarrow.so.900+0x9c921c)
#6 operator() /home/antoine/arrow/dev/cpp/src/arrow/compute/exec/exec_plan.cc:192 (libarrow.so.900+0xe82ee6)
#7 operator() /home/antoine/arrow/dev/cpp/src/arrow/util/future.h:522 (libarrow.so.900+0xea70a3)
```
I think the fix is simply to signal the condition variable with the mutex locked (which might be a bit worse performance-wise).
**Reporter**: [Antoine Pitrou](https://issues.apache.org/jira/browse/ARROW-17124) / @pitrou
**Note**: *This issue was originally created as [ARROW-17124](https://issues.apache.org/jira/browse/ARROW-17124). Please see the [migration documentation](https://github.com/apache/arrow/issues/14542) for further details.*
Contributor guide
Research direction
Start with src/arrow/util/future.cc at ConcreteFutureImpl::~ConcreteFutureImpl and DoMarkFinishedOrFailed, then inspect src/arrow/compute/exec/exec_plan.cc around ExecPlanImpl cleanup and the completion callback. Reproduce the report with ThreadSanitizer using arrow-dataset-file-test's TestFileSystemDataset_WriteProjected_Test in src/arrow/dataset/file_test.cc; done means the race no longer appears.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100