SteNicholas opened a new issue, #385:
URL: https://github.com/apache/paimon-cpp/issues/385

   ### Search before asking
   
   - [x] I searched in the 
[issues](https://github.com/apache/paimon-cpp/issues) and found nothing similar.
   
   ### Paimon-cpp version
   
   main, observed on commit dc745fe91932dc2cba919fb13639dfc427280006 (the 
change in that commit only touches `realtime/`, unrelated to this failure).
   
   ### Minimal reproduce step
   
   Flaky; observed in CI job `gcc-release-x86_64`: 
https://github.com/apache/paimon-cpp/actions/runs/35845901520/job/107134278444
   
   `paimon-parquet-format-test` crashes with SIGSEGV right after 
`VariantParquetTest.WriteAndReadRoundTrip` passes, while 
`VariantParquetTest.ShreddedWriteAndReadRoundTrip` is starting:
   
   ```
   [ RUN      ] VariantParquetTest.ShreddedWriteAndReadRoundTrip
   build_support/run-test.sh: line 98: 67868 Segmentation fault      (core 
dumped)
   Program terminated with signal SIGSEGV, Segmentation fault.
   
   Thread 1 (Arrow IO thread pool):
   #0 arrow::PoolBuffer::~PoolBuffer()
   #1 
arrow::Future<std::shared_ptr<arrow::Buffer>>::SetResult(...)::{lambda(void*)#1}::_FUN(void*)
   #2 arrow::ConcreteFutureImpl::~ConcreteFutureImpl()
   #3 std::_Sp_counted_base<...>::_M_release_last_use_cold()
   #4 arrow::internal::FnOnce<void 
()>::FnImpl<std::_Bind<arrow::detail::ContinueFuture 
(arrow::Future<std::shared_ptr<arrow::Buffer>>, 
arrow::io::RandomAccessFile::ReadAsync(arrow::io::IOContext const&, long, 
long)::{lambda()#1})>>::~FnImpl()
   #5 arrow::internal::ThreadPool::LaunchWorkersUnlocked(int)::{lambda()#1} ...
   ```
   
   The main thread is already running the next test 
(`VariantShreddingWritePlan::CreateFromPhysicalSchema`) and is unrelated to the 
crash.
   
   ### What doesn't meet your expectations?
   
   The test binary should not crash. The crash is a use-after-free of the Arrow 
memory pool in the test fixture:
   
   1. `VariantParquetTest::SetUp()` creates a per-test pool adaptor: 
`arrow_pool_ = GetArrowPool(pool_);` 
(`src/paimon/format/parquet/variant_parquet_test.cpp`). It is destroyed 
together with the fixture.
   2. Several tests (`WriteAndReadRoundTrip`, `ShreddedWriteAndReadRoundTrip`, 
and the helper around line 548) open the file for a raw sanity check with the 
plain Arrow reader:
      ```cpp
      auto file = arrow::io::ReadableFile::Open(file_path_, arrow_pool_.get());
      ::parquet::arrow::OpenFile(file.ValueOrDie(), arrow_pool_.get(), 
&raw_reader);
      ```
      Only the raw `arrow::MemoryPool*` is passed, so nothing keeps the adaptor 
alive.
   3. With Arrow 17, `ArrowReaderProperties::pre_buffer()` defaults to `true`, 
so `ReadTable` issues `RandomAccessFile::ReadAsync`. `ReadableFile` does not 
override it, so the base implementation submits `ReadAt` to the IO thread pool 
and the result `PoolBuffer` is allocated from `arrow_pool_`.
   4. The IO worker holds the last reference to the future (and therefore the 
buffer) until its task object is destroyed, which can happen after the reader 
has consumed the data and the test has finished. When the fixture is torn down 
first, `~PoolBuffer()` calls `Free()` on the already destroyed adaptor -> 
SIGSEGV.
   
   Paimon's own read path (`ArrowInputStreamAdapter::ReadAsync`) is not 
affected: it uses a callback-based implementation and already retains the pool 
with the returned buffer (#182). This issue is limited to tests that use 
`arrow::io::ReadableFile` with a raw pointer to a short-lived pool.
   
   ### Anything else?
   
   Possible fixes (test-only):
   
   - Use a process-lifetime Arrow pool for these raw sanity-check readers, e.g. 
`arrow::default_memory_pool()`, instead of the per-fixture `arrow_pool_.get()`; 
or
   - Disable pre-buffering for the raw reader 
(`ArrowReaderProperties::set_pre_buffer(false)`), so no async IO task outlives 
the test; or
   - Read through `ArrowInputStreamAdapter`, which keeps the pool alive for 
returned buffers.
   
   Other tests using `arrow::io::ReadableFile::Open(..., pool.get())` with a 
per-test pool may have the same latent issue and should be checked as well.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to