Copilot commented on code in PR #203:
URL: https://github.com/apache/paimon-cpp/pull/203#discussion_r3780655541
##########
src/paimon/common/factories/io_hook_test.cpp:
##########
@@ -64,4 +68,49 @@ TEST(IOHookTest, TestThrowExceptionMode) {
hook->Clear();
}
+// Regression test for the data race on IOHook's mode: Reset()/Clear() run on
one
+// thread while other threads call Try() concurrently. Under a ThreadSanitizer
build
+// this deterministically reports the unsynchronized mode access; functionally
it must
+// never crash and every Try() must return either OK or the injected IOError.
+TEST(IOHookTest, TestConcurrentResetAndTry) {
+ auto hook = IOHook::GetInstance();
+
+ constexpr int32_t kResetIterations = 200000;
+ constexpr int32_t kTryIterations = 50000;
+ constexpr int32_t kNumWorkers = 4;
+
+ std::atomic<bool> unexpected_status{false};
+
+ std::thread reset_thread([hook]() {
+ for (int32_t i = 0; i < kResetIterations; i++) {
+ hook->Reset(i, IOHook::Mode::RETURN_ERROR);
+ hook->Clear();
+ }
+ });
+
+ std::vector<std::thread> workers;
+ workers.reserve(kNumWorkers);
+ for (int32_t t = 0; t < kNumWorkers; t++) {
+ workers.emplace_back([hook, &unexpected_status]() {
+ for (int32_t i = 0; i < kTryIterations; i++) {
+ Status status = hook->Try("concurrent_path");
+ // Only RETURN_ERROR mode is armed here, so Try() may only
return OK or
+ // IOError; anything else means the mode was read as garbage.
Review Comment:
This comment is inaccurate: the reset thread calls `Clear()`, which sets
mode back to `SILENT`, so the workers are racing with both `RETURN_ERROR` and
`SILENT` modes. The assertion logic is fine, but the comment should reflect the
actual modes exercised so future maintainers don't misread the test’s intent.
--
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]
