You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "pitrou (via GitHub)" <gi...@apache.org> on 2023/07/07 09:03:59 UTC
[GitHub] [arrow] pitrou opened a new pull request, #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
pitrou opened a new pull request, #36536:
URL: https://github.com/apache/arrow/pull/36536
XXX need to explain this a bit
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #36536:
URL: https://github.com/apache/arrow/pull/36536#discussion_r1257281443
##########
cpp/src/arrow/filesystem/s3fs.cc:
##########
@@ -1158,7 +1179,7 @@ class ObjectInputFile final : public io::RandomAccessFile {
req.SetKey(ToAwsString(path_.key));
ARROW_ASSIGN_OR_RAISE(auto client_lock, holder_->Lock());
- auto outcome = client_lock->HeadObject(req);
+ auto outcome = std::move(client_lock)->HeadObject(req);
Review Comment:
Ok, I opted for a more manual approach because mixing `arrow::Result` and `Aws::Outcome` is not really convenient:
https://github.com/apache/arrow/pull/36536/files#diff-27b767cd06b879c40106ea98d21d3b12f13ec6e9f9aa49218d10583c5c1899c4R725-R726
```c++
S3ClientLock Move() { return std::move(*this); }
```
```c++
auto deferred = [owned_buffer, holder = holder_, req = std::move(req),
state = upload_state_,
part_number = part_number_]() mutable -> Status {
ARROW_ASSIGN_OR_RAISE(auto client_lock, holder->Lock());
auto outcome = client_lock.Move()->UploadPart(req);
HandleUploadOutcome(state, part_number, req, outcome);
return Status::OK();
};
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] westonpace commented on pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on PR #36536:
URL: https://github.com/apache/arrow/pull/36536#issuecomment-1629087854
+1, after running my test multiple times with and without CPU stress I no longer see any TSAN errors. I've included one minor comment tweak in #35440
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #36536:
URL: https://github.com/apache/arrow/pull/36536#issuecomment-1625443253
@github-actions crossbow submit -g cpp
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #36536:
URL: https://github.com/apache/arrow/pull/36536#discussion_r1256076176
##########
cpp/src/arrow/filesystem/s3fs.cc:
##########
@@ -1158,7 +1179,7 @@ class ObjectInputFile final : public io::RandomAccessFile {
req.SetKey(ToAwsString(path_.key));
ARROW_ASSIGN_OR_RAISE(auto client_lock, holder_->Lock());
- auto outcome = client_lock->HeadObject(req);
+ auto outcome = std::move(client_lock)->HeadObject(req);
Review Comment:
`std::move` will not destroy this lock (in general, it's useful to think of `std::move` as doing nothing except flagging a reference as legal to move construct from). What you want here is
```suggestion
S3Model::HeadObjectOutcome outcome;
{
ARROW_ASSIGN_OR_RAISE(auto client_lock, holder_->Lock());
outcome = std::move(client_lock)->HeadObject(req);
} // client lock goes out of scope here
```
or maybe
```suggestion
ARROW_ASSIGN_OR_RAISE(auto outcome, holder_->Lock().Map([&](auto client_lock) {
return client_lock->HeadObject(req);
}));
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #36536:
URL: https://github.com/apache/arrow/pull/36536#discussion_r1257281443
##########
cpp/src/arrow/filesystem/s3fs.cc:
##########
@@ -1158,7 +1179,7 @@ class ObjectInputFile final : public io::RandomAccessFile {
req.SetKey(ToAwsString(path_.key));
ARROW_ASSIGN_OR_RAISE(auto client_lock, holder_->Lock());
- auto outcome = client_lock->HeadObject(req);
+ auto outcome = std::move(client_lock)->HeadObject(req);
Review Comment:
Ok, I opted for a more manual approach because mixing `arrow::Result` and `Aws::Outcome` is not really convenient:
https://github.com/apache/arrow/pull/36536/files#diff-27b767cd06b879c40106ea98d21d3b12f13ec6e9f9aa49218d10583c5c1899c4R725-R726
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #36536:
URL: https://github.com/apache/arrow/pull/36536#issuecomment-1627347445
@github-actions crossbow submit -g python -g wheel -g cpp
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] conbench-apache-arrow[bot] commented on pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #36536:
URL: https://github.com/apache/arrow/pull/36536#issuecomment-1646199117
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 8eacd8c3a98c6de5b026c2a9e71aece760295ea2.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/15246790650) has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #36536:
URL: https://github.com/apache/arrow/pull/36536#issuecomment-1625130658
@github-actions crossbow submit -g cpp
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36536:
URL: https://github.com/apache/arrow/pull/36536#issuecomment-1627368576
:warning: GitHub issue #36523 **has no components**, please add labels for components.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #36536:
URL: https://github.com/apache/arrow/pull/36536#issuecomment-1627386633
@westonpace Are you available quickly for review?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36536:
URL: https://github.com/apache/arrow/pull/36536#issuecomment-1625097092
:warning: GitHub issue #36523 **has no components**, please add labels for components.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] westonpace commented on pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "westonpace (via GitHub)" <gi...@apache.org>.
westonpace commented on PR #36536:
URL: https://github.com/apache/arrow/pull/36536#issuecomment-1628994435
I'll rebase this into #35540 now and run my stress tests
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #36536:
URL: https://github.com/apache/arrow/pull/36536#discussion_r1255508280
##########
cpp/src/arrow/filesystem/s3fs.cc:
##########
@@ -1158,7 +1179,7 @@ class ObjectInputFile final : public io::RandomAccessFile {
req.SetKey(ToAwsString(path_.key));
ARROW_ASSIGN_OR_RAISE(auto client_lock, holder_->Lock());
- auto outcome = client_lock->HeadObject(req);
+ auto outcome = std::move(client_lock)->HeadObject(req);
Review Comment:
TBH I'm not sure if calling `std::move` here is enough to destroy `client_lock` at the end of this statement. @bkietz Can you advise?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #36536:
URL: https://github.com/apache/arrow/pull/36536#issuecomment-1625469769
@github-actions crossbow submit -g python -g wheel
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36536:
URL: https://github.com/apache/arrow/pull/36536#issuecomment-1625475308
Revision: 084180f6da9fc6d439398dcc63673dea8f665c14
Submitted crossbow builds: [ursacomputing/crossbow @ actions-2ec9152623](https://github.com/ursacomputing/crossbow/branches/all?query=actions-2ec9152623)
|Task|Status|
|----|------|
|test-conda-python-3.10|[](https://github.com/ursacomputing/crossbow/tree/actions-2ec9152623-github-test-conda-python-3.10)|
|test-conda-python-3.10-hdfs-2.9.2|[](https://github.com/ursacomputing/crossbow/actions/runs/5487302741/jobs/9998577829)|
|test-conda-python-3.10-hdfs-3.2.1|[](https://github.com/ursacomputing/crossbow/actions/runs/5487301106/jobs/9998573934)|
|test-conda-python-3.10-pandas-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/5487297709/jobs/9998567228)|
|test-conda-python-3.10-pandas-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/5487304572/jobs/9998582350)|
|test-conda-python-3.10-spark-master|[](https://github.com/ursacomputing/crossbow/actions/runs/5487295929/jobs/9998563085)|
|test-conda-python-3.10-substrait|[](https://github.com/ursacomputing/crossbow/actions/runs/5487297098/jobs/9998565822)|
|test-conda-python-3.11|[](https://github.com/ursacomputing/crossbow/actions/runs/5487307283/jobs/9998588171)|
|test-conda-python-3.11-dask-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/5487308705/jobs/9998591467)|
|test-conda-python-3.11-dask-upstream_devel|[](https://github.com/ursacomputing/crossbow/actions/runs/5487296855/jobs/9998565298)|
|test-conda-python-3.11-hypothesis|[](https://github.com/ursacomputing/crossbow/actions/runs/5487304004/jobs/9998581087)|
|test-conda-python-3.11-pandas-upstream_devel|[](https://github.com/ursacomputing/crossbow/actions/runs/5487302997/jobs/9998578538)|
|test-conda-python-3.8|[](https://github.com/ursacomputing/crossbow/actions/runs/5487302284/jobs/9998576733)|
|test-conda-python-3.8-pandas-1.0|[](https://github.com/ursacomputing/crossbow/actions/runs/5487302565/jobs/9998577314)|
|test-conda-python-3.8-spark-v3.1.2|[](https://github.com/ursacomputing/crossbow/actions/runs/5487310113/jobs/9998594628)|
|test-conda-python-3.9|[](https://github.com/ursacomputing/crossbow/actions/runs/5487308330/jobs/9998590325)|
|test-conda-python-3.9-pandas-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/5487300019/jobs/9998571975)|
|test-conda-python-3.9-spark-v3.2.0|[](https://github.com/ursacomputing/crossbow/actions/runs/5487296228/jobs/9998563873)|
|test-cuda-python|[](https://github.com/ursacomputing/crossbow/actions/runs/5487306657/jobs/9998586890)|
|test-debian-11-python-3|[](https://github.com/ursacomputing/crossbow/runs/14860612439)|
|test-fedora-35-python-3|[](https://github.com/ursacomputing/crossbow/runs/14860610547)|
|test-ubuntu-20.04-python-3|[](https://github.com/ursacomputing/crossbow/runs/14860600693)|
|test-ubuntu-22.04-python-3|[](https://github.com/ursacomputing/crossbow/actions/runs/5487304364/jobs/9998582055)|
|wheel-clean|[](https://github.com/ursacomputing/crossbow/actions/runs/5487304920/jobs/9998583186)|
|wheel-macos-big-sur-cp310-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487305239/jobs/9998583880)|
|wheel-macos-big-sur-cp311-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487297492/jobs/9998566677)|
|wheel-macos-big-sur-cp38-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487294674/jobs/9998559831)|
|wheel-macos-big-sur-cp39-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487307982/jobs/9998589642)|
|wheel-macos-mojave-cp310-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487295669/jobs/9998562497)|
|wheel-macos-mojave-cp311-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487303463/jobs/9998579858)|
|wheel-macos-mojave-cp38-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487301352/jobs/9998574437)|
|wheel-macos-mojave-cp39-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487309391/jobs/9998593056)|
|wheel-manylinux-2-28-cp310-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487303651/jobs/9998580382)|
|wheel-manylinux-2-28-cp310-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487302072/jobs/9998576212)|
|wheel-manylinux-2-28-cp311-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487305583/jobs/9998584571)|
|wheel-manylinux-2-28-cp311-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487306971/jobs/9998587474)|
|wheel-manylinux-2-28-cp38-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487300338/jobs/9998572611)|
|wheel-manylinux-2-28-cp38-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487301605/jobs/9998575109)|
|wheel-manylinux-2-28-cp39-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487309117/jobs/9998592337)|
|wheel-manylinux-2-28-cp39-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487300703/jobs/9998573189)|
|wheel-manylinux-2014-cp310-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487296452/jobs/9998564509)|
|wheel-manylinux-2014-cp310-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487301862/jobs/9998575627)|
|wheel-manylinux-2014-cp311-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487298391/jobs/9998568773)|
|wheel-manylinux-2014-cp311-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487303230/jobs/9998579192)|
|wheel-manylinux-2014-cp38-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487298028/jobs/9998567938)|
|wheel-manylinux-2014-cp38-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487299621/jobs/9998571358)|
|wheel-manylinux-2014-cp39-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487295132/jobs/9998561052)|
|wheel-manylinux-2014-cp39-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487307624/jobs/9998588813)|
|wheel-windows-cp310-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487306313/jobs/9998586222)|
|wheel-windows-cp311-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487305870/jobs/9998585258)|
|wheel-windows-cp38-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487294868/jobs/9998560373)|
|wheel-windows-cp39-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487294541/jobs/9998559556)|
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #36536:
URL: https://github.com/apache/arrow/pull/36536#issuecomment-1625468801
@raulcd Ideally we would have this PR in the release, because it fixes a late regression (we're not sure the regression can happen concretely, though).
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on code in PR #36536:
URL: https://github.com/apache/arrow/pull/36536#discussion_r1256146460
##########
cpp/src/arrow/filesystem/s3fs.cc:
##########
@@ -1158,7 +1179,7 @@ class ObjectInputFile final : public io::RandomAccessFile {
req.SetKey(ToAwsString(path_.key));
ARROW_ASSIGN_OR_RAISE(auto client_lock, holder_->Lock());
- auto outcome = client_lock->HeadObject(req);
+ auto outcome = std::move(client_lock)->HeadObject(req);
Review Comment:
Thanks @bkietz !
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36536:
URL: https://github.com/apache/arrow/pull/36536#issuecomment-1625133924
Revision: 69bf7b3c0c6bd228e736285ce5ffe88751112d49
Submitted crossbow builds: [ursacomputing/crossbow @ actions-fdd637d1c9](https://github.com/ursacomputing/crossbow/branches/all?query=actions-fdd637d1c9)
|Task|Status|
|----|------|
|test-alpine-linux-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5484957151/jobs/9993154188)|
|test-build-cpp-fuzz|[](https://github.com/ursacomputing/crossbow/actions/runs/5484958375/jobs/9993156528)|
|test-conda-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5484958606/jobs/9993157160)|
|test-conda-cpp-valgrind|[](https://github.com/ursacomputing/crossbow/runs/14853714678)|
|test-cuda-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5484959512/jobs/9993159373)|
|test-debian-11-cpp-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5484959220/jobs/9993158685)|
|test-debian-11-cpp-i386|[](https://github.com/ursacomputing/crossbow/actions/runs/5484958867/jobs/9993157707)|
|test-fedora-35-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5484958051/jobs/9993155902)|
|test-ubuntu-20.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5484960415/jobs/9993161728)|
|test-ubuntu-20.04-cpp-bundled|[](https://github.com/ursacomputing/crossbow/actions/runs/5484960144/jobs/9993160994)|
|test-ubuntu-20.04-cpp-minimal-with-formats|[](https://github.com/ursacomputing/crossbow/actions/runs/5484960607/jobs/9993162138)|
|test-ubuntu-20.04-cpp-thread-sanitizer|[](https://github.com/ursacomputing/crossbow/actions/runs/5484957649/jobs/9993155160)|
|test-ubuntu-22.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5484959744/jobs/9993159966)|
|test-ubuntu-22.04-cpp-20|[](https://github.com/ursacomputing/crossbow/actions/runs/5484957364/jobs/9993154616)|
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36536:
URL: https://github.com/apache/arrow/pull/36536#issuecomment-1625097064
:warning: GitHub issue #36523 **has been automatically assigned in GitHub** to PR creator.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #36536:
URL: https://github.com/apache/arrow/pull/36536#issuecomment-1625096768
Can you run this with your stress test @westonpace ?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #36536:
URL: https://github.com/apache/arrow/pull/36536#issuecomment-1629104933
Great, thank you @westonpace :-)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36536:
URL: https://github.com/apache/arrow/pull/36536#issuecomment-1625447460
Revision: 084180f6da9fc6d439398dcc63673dea8f665c14
Submitted crossbow builds: [ursacomputing/crossbow @ actions-83631051fd](https://github.com/ursacomputing/crossbow/branches/all?query=actions-83631051fd)
|Task|Status|
|----|------|
|test-alpine-linux-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5487125721/jobs/9998146231)|
|test-build-cpp-fuzz|[](https://github.com/ursacomputing/crossbow/actions/runs/5487127362/jobs/9998150335)|
|test-conda-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5487125504/jobs/9998145815)|
|test-conda-cpp-valgrind|[](https://github.com/ursacomputing/crossbow/runs/14860071927)|
|test-cuda-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5487126372/jobs/9998147875)|
|test-debian-11-cpp-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5487125928/jobs/9998146689)|
|test-debian-11-cpp-i386|[](https://github.com/ursacomputing/crossbow/actions/runs/5487128601/jobs/9998153653)|
|test-fedora-35-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5487126697/jobs/9998148528)|
|test-ubuntu-20.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5487128411/jobs/9998153101)|
|test-ubuntu-20.04-cpp-bundled|[](https://github.com/ursacomputing/crossbow/actions/runs/5487127800/jobs/9998151396)|
|test-ubuntu-20.04-cpp-minimal-with-formats|[](https://github.com/ursacomputing/crossbow/actions/runs/5487127135/jobs/9998149681)|
|test-ubuntu-20.04-cpp-thread-sanitizer|[](https://github.com/ursacomputing/crossbow/actions/runs/5487128034/jobs/9998151983)|
|test-ubuntu-22.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5487128289/jobs/9998152728)|
|test-ubuntu-22.04-cpp-20|[](https://github.com/ursacomputing/crossbow/actions/runs/5487126140/jobs/9998147219)|
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #36536:
URL: https://github.com/apache/arrow/pull/36536#issuecomment-1627352637
Revision: 85a014431febd3a215fbb0c13d2c16c390a93360
Submitted crossbow builds: [ursacomputing/crossbow @ actions-1735f0c98a](https://github.com/ursacomputing/crossbow/branches/all?query=actions-1735f0c98a)
|Task|Status|
|----|------|
|test-alpine-linux-cpp|[](https://github.com/ursacomputing/crossbow/tree/actions-1735f0c98a-github-test-alpine-linux-cpp)|
|test-build-cpp-fuzz|[](https://github.com/ursacomputing/crossbow/actions/runs/5494953682/jobs/10014037844)|
|test-conda-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5494955416/jobs/10014040722)|
|test-conda-cpp-valgrind|[](https://github.com/ursacomputing/crossbow/runs/14880127464)|
|test-conda-python-3.10|[](https://github.com/ursacomputing/crossbow/actions/runs/5494949571/jobs/10014031480)|
|test-conda-python-3.10-hdfs-2.9.2|[](https://github.com/ursacomputing/crossbow/actions/runs/5494951249/jobs/10014034148)|
|test-conda-python-3.10-hdfs-3.2.1|[](https://github.com/ursacomputing/crossbow/actions/runs/5494955250/jobs/10014040367)|
|test-conda-python-3.10-pandas-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/5494956155/jobs/10014041570)|
|test-conda-python-3.10-pandas-nightly|[](https://github.com/ursacomputing/crossbow/actions/runs/5494954909/jobs/10014039712)|
|test-conda-python-3.10-spark-master|[](https://github.com/ursacomputing/crossbow/actions/runs/5494952569/jobs/10014035950)|
|test-conda-python-3.10-substrait|[](https://github.com/ursacomputing/crossbow/actions/runs/5494955349/jobs/10014040571)|
|test-conda-python-3.11|[](https://github.com/ursacomputing/crossbow/actions/runs/5494951398/jobs/10014034342)|
|test-conda-python-3.11-dask-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/5494952961/jobs/10014036512)|
|test-conda-python-3.11-dask-upstream_devel|[](https://github.com/ursacomputing/crossbow/actions/runs/5494954425/jobs/10014038977)|
|test-conda-python-3.11-hypothesis|[](https://github.com/ursacomputing/crossbow/actions/runs/5494953220/jobs/10014037110)|
|test-conda-python-3.11-pandas-upstream_devel|[](https://github.com/ursacomputing/crossbow/actions/runs/5494955856/jobs/10014041150)|
|test-conda-python-3.8|[](https://github.com/ursacomputing/crossbow/actions/runs/5494953945/jobs/10014038203)|
|test-conda-python-3.8-pandas-1.0|[](https://github.com/ursacomputing/crossbow/actions/runs/5494949695/jobs/10014031707)|
|test-conda-python-3.8-spark-v3.1.2|[](https://github.com/ursacomputing/crossbow/actions/runs/5494949734/jobs/10014031815)|
|test-conda-python-3.9|[](https://github.com/ursacomputing/crossbow/actions/runs/5494950870/jobs/10014033668)|
|test-conda-python-3.9-pandas-latest|[](https://github.com/ursacomputing/crossbow/actions/runs/5494952226/jobs/10014035420)|
|test-conda-python-3.9-spark-v3.2.0|[](https://github.com/ursacomputing/crossbow/actions/runs/5494955946/jobs/10014041274)|
|test-cuda-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5494952666/jobs/10014036088)|
|test-cuda-python|[](https://github.com/ursacomputing/crossbow/actions/runs/5494955784/jobs/10014041065)|
|test-debian-11-cpp-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494953364/jobs/10014037354)|
|test-debian-11-cpp-i386|[](https://github.com/ursacomputing/crossbow/actions/runs/5494950295/jobs/10014032808)|
|test-debian-11-python-3|[](https://github.com/ursacomputing/crossbow/runs/14880126574)|
|test-fedora-35-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5494955162/jobs/10014040181)|
|test-fedora-35-python-3|[](https://github.com/ursacomputing/crossbow/runs/14880126793)|
|test-ubuntu-20.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5494954309/jobs/10014038789)|
|test-ubuntu-20.04-cpp-bundled|[](https://github.com/ursacomputing/crossbow/actions/runs/5494954177/jobs/10014038592)|
|test-ubuntu-20.04-cpp-minimal-with-formats|[](https://github.com/ursacomputing/crossbow/actions/runs/5494953457/jobs/10014037499)|
|test-ubuntu-20.04-cpp-thread-sanitizer|[](https://github.com/ursacomputing/crossbow/actions/runs/5494952374/jobs/10014035646)|
|test-ubuntu-20.04-python-3|[](https://github.com/ursacomputing/crossbow/runs/14880130836)|
|test-ubuntu-22.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/5494949287/jobs/10014030950)|
|test-ubuntu-22.04-cpp-20|[](https://github.com/ursacomputing/crossbow/actions/runs/5494949089/jobs/10014030663)|
|test-ubuntu-22.04-python-3|[](https://github.com/ursacomputing/crossbow/actions/runs/5494955450/jobs/10014040808)|
|wheel-clean|[](https://github.com/ursacomputing/crossbow/actions/runs/5494953290/jobs/10014037272)|
|wheel-macos-big-sur-cp310-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494951093/jobs/10014033981)|
|wheel-macos-big-sur-cp311-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494953139/jobs/10014036929)|
|wheel-macos-big-sur-cp38-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494955219/jobs/10014040293)|
|wheel-macos-big-sur-cp39-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494952473/jobs/10014035830)|
|wheel-macos-mojave-cp310-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494953555/jobs/10014037674)|
|wheel-macos-mojave-cp311-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494955314/jobs/10014040503)|
|wheel-macos-mojave-cp38-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494950427/jobs/10014033013)|
|wheel-macos-mojave-cp39-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494950019/jobs/10014032387)|
|wheel-manylinux-2-28-cp310-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494949179/jobs/10014030772)|
|wheel-manylinux-2-28-cp310-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494949944/jobs/10014032249)|
|wheel-manylinux-2-28-cp311-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494949790/jobs/10014031928)|
|wheel-manylinux-2-28-cp311-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494954606/jobs/10014039264)|
|wheel-manylinux-2-28-cp38-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494955645/jobs/10014040909)|
|wheel-manylinux-2-28-cp38-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494950681/jobs/10014033391)|
|wheel-manylinux-2-28-cp39-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494955016/jobs/10014039908)|
|wheel-manylinux-2-28-cp39-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494952889/jobs/10014036385)|
|wheel-manylinux-2014-cp310-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494954759/jobs/10014039433)|
|wheel-manylinux-2014-cp310-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494951763/jobs/10014034829)|
|wheel-manylinux-2014-cp311-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494952082/jobs/10014035248)|
|wheel-manylinux-2014-cp311-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494954098/jobs/10014038438)|
|wheel-manylinux-2014-cp38-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494951606/jobs/10014034644)|
|wheel-manylinux-2014-cp38-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494953080/jobs/10014036777)|
|wheel-manylinux-2014-cp39-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494950520/jobs/10014033186)|
|wheel-manylinux-2014-cp39-arm64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494950136/jobs/10014032598)|
|wheel-windows-cp310-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494956058/jobs/10014041421)|
|wheel-windows-cp311-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494949379/jobs/10014031124)|
|wheel-windows-cp38-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494953804/jobs/10014038037)|
|wheel-windows-cp39-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/5494955110/jobs/10014040074)|
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou merged pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou merged PR #36536:
URL: https://github.com/apache/arrow/pull/36536
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a diff in pull request #36536: GH-36523: [C++] Fix TSan-detected lock ordering issues in S3
Posted by "bkietz (via GitHub)" <gi...@apache.org>.
bkietz commented on code in PR #36536:
URL: https://github.com/apache/arrow/pull/36536#discussion_r1256088406
##########
cpp/src/arrow/filesystem/s3fs.cc:
##########
@@ -1158,7 +1179,7 @@ class ObjectInputFile final : public io::RandomAccessFile {
req.SetKey(ToAwsString(path_.key));
ARROW_ASSIGN_OR_RAISE(auto client_lock, holder_->Lock());
- auto outcome = client_lock->HeadObject(req);
+ auto outcome = std::move(client_lock)->HeadObject(req);
Review Comment:
Or you might even prefer
```c++
template <auto MemberPtr, typename... Args>
auto S3ClientHolder::LockThenCall(Args&&... args) {
return Lock().Map([&](S3ClientLock client_lock) {
return (client_lock.get()->*MemberPtr)(std::forward<Args>(args)...);
});
}
```
used like
```suggestion
ARROW_ASSIGN_OR_RAISE(auto outcome, holder_->LockThenCall<&S3Client::HeadObject>(req));
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org