Skip to content

Commit 6657a92

Browse files
authored
GH-48799: [C++] Improve SharedExclusiveChecker error messages (#48800)
### Rationale for this change The XXX comment in `SharedExclusiveChecker` noted that error messages were too generic and didn't describe the actual operations involved. **Shared lock:** `ReadAt()`: https://github.com/apache/arrow/blob/7be5a89ef083f38317fd94330127be5b5df648d8/cpp/src/arrow/io/concurrency.h#L210-L212 https://github.com/apache/arrow/blob/7be5a89ef083f38317fd94330127be5b5df648d8/cpp/src/arrow/io/concurrency.h#L215-L217 `GetSize()`: https://github.com/apache/arrow/blob/7be5a89ef083f38317fd94330127be5b5df648d8/cpp/src/arrow/io/concurrency.h#L199-L201 **Exclusive lock:** `Read()`: https://github.com/apache/arrow/blob/7be5a89ef083f38317fd94330127be5b5df648d8/cpp/src/arrow/io/concurrency.h#L109-L111 https://github.com/apache/arrow/blob/7be5a89ef083f38317fd94330127be5b5df648d8/cpp/src/arrow/io/concurrency.h#L114-L116 https://github.com/apache/arrow/blob/7be5a89ef083f38317fd94330127be5b5df648d8/cpp/src/arrow/io/concurrency.h#L179-L181 https://github.com/apache/arrow/blob/7be5a89ef083f38317fd94330127be5b5df648d8/cpp/src/arrow/io/concurrency.h#L184-L186 `Seek()`: https://github.com/apache/arrow/blob/7be5a89ef083f38317fd94330127be5b5df648d8/cpp/src/arrow/io/concurrency.h#L194-L196 `Tell()`: https://github.com/apache/arrow/blob/7be5a89ef083f38317fd94330127be5b5df648d8/cpp/src/arrow/io/concurrency.h#L104-L106 https://github.com/apache/arrow/blob/7be5a89ef083f38317fd94330127be5b5df648d8/cpp/src/arrow/io/concurrency.h#L174-L176 `Peek()`: https://github.com/apache/arrow/blob/7be5a89ef083f38317fd94330127be5b5df648d8/cpp/src/arrow/io/concurrency.h#L119-L121 https://github.com/apache/arrow/blob/7be5a89ef083f38317fd94330127be5b5df648d8/cpp/src/arrow/io/concurrency.h#L189-L191 `Close()`: https://github.com/apache/arrow/blob/7be5a89ef083f38317fd94330127be5b5df648d8/cpp/src/arrow/io/concurrency.h#L94-L96 https://github.com/apache/arrow/blob/7be5a89ef083f38317fd94330127be5b5df648d8/cpp/src/arrow/io/concurrency.h#L164-L166 `Abort()`: https://github.com/apache/arrow/blob/7be5a89ef083f38317fd94330127be5b5df648d8/cpp/src/arrow/io/concurrency.h#L99-L101 https://github.com/apache/arrow/blob/7be5a89ef083f38317fd94330127be5b5df648d8/cpp/src/arrow/io/concurrency.h#L169-L171 ### What changes are included in this PR? Improved three error messages in `SharedExclusiveChecker` to explicitly list the I/O operations involved: - Shared lock operations: `ReadAt`, `GetSize` - Exclusive lock operations: `Read`, `Seek`, `Tell`, `Peek`, `Close`, `Abort` ### Are these changes tested? I manually tested all of combinations. ### Are there any user-facing changes? No. * GitHub Issue: #48799 Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Antoine Pitrou <antoine@python.org>
1 parent 985b16e commit 6657a92

File tree

1 file changed

+6
-5
lines changed

1 file changed

+6
-5
lines changed

cpp/src/arrow/io/interfaces.cc

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -338,10 +338,9 @@ SharedExclusiveChecker::SharedExclusiveChecker() : impl_(new Impl) {}
338338

339339
void SharedExclusiveChecker::LockShared() {
340340
std::lock_guard<std::mutex> lock(impl_->mutex);
341-
// XXX The error message doesn't really describe the actual situation
342-
// (e.g. ReadAt() called while Read() call in progress)
343341
ARROW_CHECK_EQ(impl_->n_exclusive, 0)
344-
<< "Attempted to take shared lock while locked exclusive";
342+
<< "Cannot perform position-independent I/O (ReadAt/GetSize) while an "
343+
"exclusive operation (Read/Seek/Tell/Peek/Close/Abort) is in progress";
345344
++impl_->n_shared;
346345
}
347346

@@ -354,9 +353,11 @@ void SharedExclusiveChecker::UnlockShared() {
354353
void SharedExclusiveChecker::LockExclusive() {
355354
std::lock_guard<std::mutex> lock(impl_->mutex);
356355
ARROW_CHECK_EQ(impl_->n_shared, 0)
357-
<< "Attempted to take exclusive lock while locked shared";
356+
<< "Cannot perform exclusive I/O operation (Read/Seek/Tell/Peek/Close/Abort) while "
357+
"position-independent operations (ReadAt/GetSize) are in progress";
358358
ARROW_CHECK_EQ(impl_->n_exclusive, 0)
359-
<< "Attempted to take exclusive lock while already locked exclusive";
359+
<< "Cannot perform exclusive I/O operation (Read/Seek/Tell/Peek/Close/Abort) while "
360+
"another exclusive operation is already in progress";
360361
++impl_->n_exclusive;
361362
}
362363

0 commit comments

Comments
 (0)