Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Data race in setting the shutdown_callback #469

Closed
1 task done
grrtrr opened this issue Dec 2, 2024 · 2 comments · Fixed by #470
Closed
1 task done

Data race in setting the shutdown_callback #469

grrtrr opened this issue Dec 2, 2024 · 2 comments · Fixed by #470
Labels
bug Something isn't working p1 This is a high priority issue potential-regression Marking this issue as a potential regression to be checked by team member

Comments

@grrtrr
Copy link
Contributor

grrtrr commented Dec 2, 2024

Describe the bug

Data race in setting the shutdown_callback.

Appears to be introduced by #440.

Regression Issue

  • Select this option if this issue appears to be a regression.

Expected Behavior

No data race.

Current Behavior

Data race happens (with v0.7.0):

  Read of size 8 at 0x7b5c00033720 by thread T4:
    #0 s_s3_meta_request_destroy external/aws-c-s3/source/s3_meta_request.c:498:72 (libexternal_Saws-c-s3_Slibaws-c-s3.so+0x47c43)
    #1 aws_ref_count_release external/aws-c-common/source/ref_count.c:29:9 (libexternal_Saws-c-common_Slibaws-c-common.so+0x85805)
    #2 aws_s3_meta_request_release external/aws-c-s3/source/s3_meta_request.c:480:9 (libexternal_Saws-c-s3_Slibaws-c-s3.so+0x49324)
    #3 s_s3_client_remove_meta_request_threaded external/aws-c-s3/source/s3_client.c:1567:5 (libexternal_Saws-c-s3_Slibaws-c-s3.so+0x37504)
    #4 aws_s3_client_update_meta_requests_threaded external/aws-c-s3/source/s3_client.c:1912:17 (libexternal_Saws-c-s3_Slibaws-c-s3.so+0x36f01)
    #5 s_s3_client_process_work_default external/aws-c-s3/source/s3_client.c:1686:9 (libexternal_Saws-c-s3_Slibaws-c-s3.so+0x3b78e)
    #6 s_s3_client_process_work_task external/aws-c-s3/source/s3_client.c:1584:5 (libexternal_Saws-c-s3_Slibaws-c-s3.so+0x3c85d)
    #7 aws_task_run external/aws-c-common/source/task_scheduler.c:44:5 (libexternal_Saws-c-common_Slibaws-c-common.so+0x88a16)
    #8 s_run_all external/aws-c-common/source/task_scheduler.c:249:9 (libexternal_Saws-c-common_Slibaws-c-common.so+0x89277)
    #9 aws_task_scheduler_run_all external/aws-c-common/source/task_scheduler.c:188:5 (libexternal_Saws-c-common_Slibaws-c-common.so+0x89ac7)
    #10 aws_event_loop_thread external/aws-c-io/source/linux/epoll_event_loop.c:666:9 (libexternal_Saws-c-io_Slibaws-c-io.so+0x4d075)
    #11 thread_fn external/aws-c-common/source/posix/thread.c:177:5 (libexternal_Saws-c-common_Slibaws-c-common.so+0x81d53)

  Previous write of size 8 at 0x7b5c00033720 by main thread:
    #0 aws_s3_client_make_meta_request external/aws-c-s3/source/s3_client.c:1152:41 (libexternal_Saws-c-s3_Slibaws-c-s3.so+0x36a52)
    #1 Aws::S3Crt::S3CrtClient::PutObjectAsync(Aws::S3Crt::Model::PutObjectRequest const&, std::function<void (Aws::S3Crt::S3CrtClient const*, Aws::S3Crt::Model::PutObjectRequest const&, Aws::Utils::Outcome<Aws::S3Crt::Model::PutObjectResult, Aws::S3Crt::S3CrtError> const&, std::shared_ptr<Aws::Client::AsyncCallerContext const> const&)> const&, std::shared_ptr<Aws::Client::AsyncCallerContext const> const&) const external/com_github_aws-sdk-cpp/generated/src/aws-cpp-sdk-s3-crt/source/S3CrtClient.cpp:673:7 (libexternal_Scom_Ugithub_Uaws-sdk-cpp_Slibaws-s3-crt.so+0x1b5c31)
    #2 Aws::S3Crt::S3CrtClient::PutObject(Aws::S3Crt::Model::PutObjectRequest const&) const external/com_github_aws-sdk-cpp/generated/src/aws-cpp-sdk-s3-crt/source/S3CrtClient.cpp:689:16 (libexternal_Scom_Ugithub_Uaws-sdk-cpp_Slibaws-s3-crt.so+0x1b61d0)
    #3 av::cloud::aws::s3::S3Ostreambuf::PutObject(std::basic_streambuf<char, std::char_traits<char>>*) cloud/aws/s3/s3_streambuf.cc:98:35 (libcloud_Saws_Ss3_Slibs3_Ustreambuf.so+0x33e26)
    #4 av::cloud::aws::s3::S3Ostream::PutObject(std::basic_streambuf<char, std::char_traits<char>>*) cloud/aws/s3/s3_stream.hh:23:69 (libcloud_Saws_Ss3_Slibs3_Ublobstore_Uplugin.so+0x91f79)
    #5 av::cloud::aws::s3::S3OutputBlob::write_and_close(char const*, unsigned long) cloud/aws/s3/s3_blobstore_plugin.cc:344:23 (libcloud_Saws_Ss3_Slibs3_Ublobstore_Uplugin.so+0x8812c)
    #6 av::blobstore::write_blob(av::blobstore::WriteMetadata const&, char const*, unsigned long) common/blobstore/blobstore.cc:85:20 (libcommon_Sblobstore_Slibblobstore_Uno_Ulocal.so+0x7c554)
    #7 av::blobstore::write_blob(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::basic_string_view<char, std::char_traits<char>>) common/blobstore/blobstore.cc:89:12 (libcommon_Sblobstore_Slibblobstore_Uno_Ulocal.so+0x7c848)
    #8 av::blobstore::(anonymous namespace)::S3BlobstorePluginTest_SmokeTest_Test::TestBody() cloud/aws/s3/s3_blobstore_plugin_test.cc:41:44 (s3_blobstore_plugin_test+0x175916)
    #9 void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) external/com_google_googletest/googletest/src/gtest.cc:2580:10 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x15195c)
    #10 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) external/com_google_googletest/googletest/src/gtest.cc:2616:14 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x13308a)
    #11 testing::Test::Run() external/com_google_googletest/googletest/src/gtest.cc:2655:5 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x114a38)
    #12 testing::TestInfo::Run() external/com_google_googletest/googletest/src/gtest.cc:2832:11 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x1159dd)
    #13 testing::TestSuite::Run() external/com_google_googletest/googletest/src/gtest.cc:2986:28 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x1165a0)
    #14 testing::internal::UnitTestImpl::RunAllTests() external/com_google_googletest/googletest/src/gtest.cc:5697:44 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x127184)
    #15 bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) external/com_google_googletest/googletest/src/gtest.cc:2580:10 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x157b0f)
    #16 bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) external/com_google_googletest/googletest/src/gtest.cc:2616:14 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x136faa)
    #17 testing::UnitTest::Run() external/com_google_googletest/googletest/src/gtest.cc:5280:10 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x1269ed)
    #18 RUN_ALL_TESTS() external/com_google_googletest/googletest/include/gtest/gtest.h:2485:46 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest_Umain.so+0x2057)
    #19 main external/com_google_googletest/googlemock/src/gmock_main.cc:70:10 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest_Umain.so+0x1f64)

  Location is heap block of size 824 at 0x7b5c00033680 allocated by main thread:
    #0 malloc /var/lib/buildkite-agent/.cache/avos/_avos_buildkite-agent/aurora-x86-kirkstone-nuc-toolchain/build/tmp/work-shared/llvm-project-source-15.0.7-r0/git/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:667:5 (s3_blobstore_plugin_test+0xf4831)
    #1 Aws::Malloc(char const*, unsigned long) external/com_github_aws-sdk-cpp/src/aws-cpp-sdk-core/source/utils/memory/AWSMemory.cpp:76:21 (libexternal_Scom_Ugithub_Uaws-sdk-cpp_Slibaws-core.so+0x45915a)
    #2 Aws::MemAcquire(aws_allocator*, unsigned long) external/com_github_aws-sdk-cpp/src/aws-cpp-sdk-core/source/utils/memory/AWSMemory.cpp:104:12 (libexternal_Scom_Ugithub_Uaws-sdk-cpp_Slibaws-core.so+0x459398)
    #3 aws_mem_calloc external/aws-c-common/source/allocator.c:203:17 (libexternal_Saws-c-common_Slibaws-c-common.so+0x2e261)
    #4 aws_s3_meta_request_default_new external/aws-c-s3/source/s3_default_meta_request.c:86:9 (libexternal_Saws-c-s3_Slibaws-c-s3.so+0x424db)
    #5 s_s3_client_meta_request_factory_default external/aws-c-s3/source/s3_client.c:1341:28 (libexternal_Saws-c-s3_Slibaws-c-s3.so+0x3aaa8)
    #6 aws_s3_client_make_meta_request external/aws-c-s3/source/s3_client.c:1019:48 (libexternal_Saws-c-s3_Slibaws-c-s3.so+0x35daf)
    #7 Aws::S3Crt::S3CrtClient::PutObjectAsync(Aws::S3Crt::Model::PutObjectRequest const&, std::function<void (Aws::S3Crt::S3CrtClient const*, Aws::S3Crt::Model::PutObjectRequest const&, Aws::Utils::Outcome<Aws::S3Crt::Model::PutObjectResult, Aws::S3Crt::S3CrtError> const&, std::shared_ptr<Aws::Client::AsyncCallerContext const> const&)> const&, std::shared_ptr<Aws::Client::AsyncCallerContext const> const&) const external/com_github_aws-sdk-cpp/generated/src/aws-cpp-sdk-s3-crt/source/S3CrtClient.cpp:673:7 (libexternal_Scom_Ugithub_Uaws-sdk-cpp_Slibaws-s3-crt.so+0x1b5c31)
    #8 Aws::S3Crt::S3CrtClient::PutObject(Aws::S3Crt::Model::PutObjectRequest const&) const external/com_github_aws-sdk-cpp/generated/src/aws-cpp-sdk-s3-crt/source/S3CrtClient.cpp:689:16 (libexternal_Scom_Ugithub_Uaws-sdk-cpp_Slibaws-s3-crt.so+0x1b61d0)
    #9 av::cloud::aws::s3::S3Ostreambuf::PutObject(std::basic_streambuf<char, std::char_traits<char>>*) cloud/aws/s3/s3_streambuf.cc:98:35 (libcloud_Saws_Ss3_Slibs3_Ustreambuf.so+0x33e26)
    #10 av::cloud::aws::s3::S3Ostream::PutObject(std::basic_streambuf<char, std::char_traits<char>>*) cloud/aws/s3/s3_stream.hh:23:69 (libcloud_Saws_Ss3_Slibs3_Ublobstore_Uplugin.so+0x91f79)
    #11 av::cloud::aws::s3::S3OutputBlob::write_and_close(char const*, unsigned long) cloud/aws/s3/s3_blobstore_plugin.cc:344:23 (libcloud_Saws_Ss3_Slibs3_Ublobstore_Uplugin.so+0x8812c)
    #12 av::blobstore::write_blob(av::blobstore::WriteMetadata const&, char const*, unsigned long) common/blobstore/blobstore.cc:85:20 (libcommon_Sblobstore_Slibblobstore_Uno_Ulocal.so+0x7c554)
    #13 av::blobstore::write_blob(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::basic_string_view<char, std::char_traits<char>>) common/blobstore/blobstore.cc:89:12 (libcommon_Sblobstore_Slibblobstore_Uno_Ulocal.so+0x7c848)
    #14 av::blobstore::(anonymous namespace)::S3BlobstorePluginTest_SmokeTest_Test::TestBody() cloud/aws/s3/s3_blobstore_plugin_test.cc:41:44 (s3_blobstore_plugin_test+0x175916)
    #15 void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) external/com_google_googletest/googletest/src/gtest.cc:2580:10 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x15195c)
    #16 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) external/com_google_googletest/googletest/src/gtest.cc:2616:14 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x13308a)
    #17 testing::Test::Run() external/com_google_googletest/googletest/src/gtest.cc:2655:5 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x114a38)
    #18 testing::TestInfo::Run() external/com_google_googletest/googletest/src/gtest.cc:2832:11 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x1159dd)
    #19 testing::TestSuite::Run() external/com_google_googletest/googletest/src/gtest.cc:2986:28 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x1165a0)
    #20 testing::internal::UnitTestImpl::RunAllTests() external/com_google_googletest/googletest/src/gtest.cc:5697:44 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x127184)
    #21 bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) external/com_google_googletest/googletest/src/gtest.cc:2580:10 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x157b0f)
    #22 bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) external/com_google_googletest/googletest/src/gtest.cc:2616:14 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x136faa)
    #23 testing::UnitTest::Run() external/com_google_googletest/googletest/src/gtest.cc:5280:10 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x1269ed)
    #24 RUN_ALL_TESTS() external/com_google_googletest/googletest/include/gtest/gtest.h:2485:46 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest_Umain.so+0x2057)
    #25 main external/com_google_googletest/googlemock/src/gmock_main.cc:70:10 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest_Umain.so+0x1f64)

Reproduction Steps

Does not appear when reverting #440.

Possible Solution

The callback is set outside of the critical region. It should be set inside.

Additional Information/Context

No response

aws-c-s3 version used

v0.7.0

Compiler and version used

clang-15

Operating System and version

ubuntu 22.04
@grrtrr grrtrr added bug Something isn't working needs-triage This issue or PR still needs to be triaged. labels Dec 2, 2024
@github-actions github-actions bot added the potential-regression Marking this issue as a potential regression to be checked by team member label Dec 2, 2024
@grrtrr grrtrr mentioned this issue Dec 2, 2024
Merged
@waahm7
Copy link
Contributor

waahm7 commented Dec 2, 2024

Thank you for creating the issue and the PR, and we are looking into how this race condition gets triggered. Are you calling aws_s3_meta_request_release in any callbacks, like the on_finish callback?

@grrtrr
Copy link
Contributor Author

grrtrr commented Dec 3, 2024 •
edited
Loading

Yes we are calling it in S3CrtRequestFinishCallback of the S3CrtClient in the AWS C++ SDK.

@jmklix jmklix added p1 This is a high priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working p1 This is a high priority issue potential-regression Marking this issue as a potential regression to be checked by team member
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[meta request]: assign shutdown_callback inside critical region grrtrr/aws-c-s3
3 participants
@grrtrr @jmklix @waahm7