[aws-cpp-sdk-core] undefined behaviour when flushing ResponseStream before deletion

[grrtrr]

Problem description

We encountered segmentation faults after switching from fstream to in-memory downloads (stringstream, boost vectorstream). It seems the original condition in #58 was not fixed. A related issue (looking at a different aspect) is #1732.

In our downloads we set the rdbuf() member of the Aws::IOStream (std::iostream) returned by the ResponseStreamFactory of the GetRequest, in order to reduce copying.

This would not be a problem since the std::iostream destructor does not destroy the associated rdbuf().

However, the flush() operation in ~ResponseStream, which occurs after the download completed, uses the associated rdbuf() (which by now has gone out of scope), as shown in the following core dump.

Core dump

(gdb) bt
#0  0x00007f0e7b41bc4b in std::ostream::flush() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#1  0x00007f0e78e5888d in Aws::Utils::Stream::ResponseStream::ReleaseStream (this=0x7f0e477fc2b0) at external/com_github_aws-sdk-cpp/aws-cpp-sdk-core/source/utils/stream/ResponseStream.cpp:61
#2  0x00007f0e78e5885e in Aws::Utils::Stream::ResponseStream::~ResponseStream (this=0x7f0e477fc2b0, __in_chrg=<optimized out>) at external/com_github_aws-sdk-cpp/aws-cpp-sdk-core/source/utils/stream/ResponseStream.cpp:54
#3  0x00007f0e786714a8 in Aws::S3Crt::Model::GetObjectResult::~GetObjectResult (this=0x7f0e477fc2b0, __in_chrg=<optimized out>) at external/com_github_aws-sdk-cpp/aws-cpp-sdk-s3-crt/include/aws/s3-crt/model/GetObjectResult.h:30
#4  0x00007f0e78671ede in Aws::Utils::Outcome<Aws::S3Crt::Model::GetObjectResult, Aws::S3Crt::S3CrtError>::~Outcome (this=0x7f0e477fc2b0, __in_chrg=<optimized out>) at external/com_github_aws-sdk-cpp/aws-cpp-sdk-core/include/aws/core/utils/Outcome.h:25
#5  0x00007f0e78a78cd5 in Aws::TransferCrt::TransferManager::<lambda(const Aws::S3Crt::S3CrtClient*, const Aws::S3Crt::Model::GetObjectRequest&, Aws::S3Crt::Model::GetObjectOutcome, const std::shared_ptr<const Aws::Client::AsyncCallerContext>&)>::operator()(const Aws::S3Crt::S3CrtClient *, const Aws::S3Crt::Model::GetObjectRequest &, Aws::S3Crt::Model::GetObjectOutcome, const std::shared_ptr<Aws::Client::AsyncCallerContext const> &) const (__closure=0x55e51771be00, client=0x55e5175d9a30, request=..., outcome=..., 
    context=std::shared_ptr<const Aws::Client::AsyncCallerContext> (use count 3, weak count 0) = {...}) at third_party/com_github_aws_sdk_cpp/transfer_manager/TransferManager.cc:270
#6  0x00007f0e78a7b4df in std::_Function_handler<void(const Aws::S3Crt::S3CrtClient*, const Aws::S3Crt::Model::GetObjectRequest&, Aws::Utils::Outcome<Aws::S3Crt::Model::GetObjectResult, Aws::S3Crt::S3CrtError>, const std::shared_ptr<const Aws::Client::AsyncCallerContext>&), Aws::TransferCrt::TransferManager::GetObject(const std::shared_ptr<Aws::TransferCrt::DownloadContext>&, const std::shared_ptr<Aws::TransferCrt::TransferHandle>&)::<lambda(const Aws::S3Crt::S3CrtClient*, const Aws::S3Crt::Model::GetObjectRequest&, Aws::S3Crt::Model::GetObjectOutcome, const std::shared_ptr<const Aws::Client::AsyncCallerContext>&)> >::_M_invoke(const std::_Any_data &, const Aws::S3Crt::S3CrtClient *&&, const Aws::S3Crt::Model::GetObjectRequest &, Aws::Utils::Outcome<Aws::S3Crt::Model::GetObjectResult, Aws::S3Crt::S3CrtError> &&, const std::shared_ptr<Aws::Client::AsyncCallerContext const> &) (__functor=..., __args#0=@0x7f0e477fcc10: 0x55e5175d9a30, __args#1=..., __args#2=..., __args#3=std::shared_ptr<const Aws::Client::AsyncCallerContext> (use count 3, weak count 0) = {...}) at /usr/include/c++/7/bits/std_function.h:316
#7  0x00007f0e78c0770e in std::function<void (Aws::S3Crt::S3CrtClient const*, Aws::S3Crt::Model::GetObjectRequest const&, Aws::Utils::Outcome<Aws::S3Crt::Model::GetObjectResult, Aws::S3Crt::S3CrtError>, std::shared_ptr<Aws::Client::AsyncCallerContext const> const&)>::operator()(Aws::S3Crt::S3CrtClient const*, Aws::S3Crt::Model::GetObjectRequest const&, Aws::Utils::Outcome<Aws::S3Crt::Model::GetObjectResult, Aws::S3Crt::S3CrtError>, std::shared_ptr<Aws::Client::AsyncCallerContext const> const&) const (this=0x55e517572828, __args#0=0x55e5175d9a30, __args#1=..., 
    __args#2=..., __args#3=std::shared_ptr<const Aws::Client::AsyncCallerContext> (use count 3, weak count 0) = {...}) at /usr/include/c++/7/bits/std_function.h:706
#8  0x00007f0e78a8c67b in GetObjectRequestShutdownCallback (user_data=0x55e517572820) at external/com_github_aws-sdk-cpp/aws-cpp-sdk-s3-crt/source/S3CrtClient.cpp:512
#9  0x00007f0e78ecc49f in s_s3_meta_request_destroy (user_data=0x55e51772f1c0) at external/aws-c-s3/source/s3_meta_request.c:423
#10 0x00007f0e790208cd in aws_ref_count_release (ref_count=0x55e51772f1c8) at external/aws-c-common/source/ref_count.c:29
#11 0x00007f0e78ecc286 in aws_s3_meta_request_release (meta_request=0x55e51772f1c0) at external/aws-c-s3/source/s3_meta_request.c:385
#12 0x00007f0e78ec5273 in s_s3_client_remove_meta_request_threaded (client=0x55e5176fada0, meta_request=0x55e51772f1c0) at external/aws-c-s3/source/s3_client.c:1071
#13 0x00007f0e78ec5cc0 in aws_s3_client_update_meta_requests_threaded (client=0x55e5176fada0) at external/aws-c-s3/source/s3_client.c:1363
#14 0x00007f0e78ec5640 in s_s3_client_process_work_default (client=0x55e5176fada0) at external/aws-c-s3/source/s3_client.c:1180
#15 0x00007f0e78ec52a6 in s_s3_client_process_work_task (task=0x55e5176faf38, arg=0x55e5176fada0, task_status=AWS_TASK_STATUS_RUN_READY) at external/aws-c-s3/source/s3_client.c:1088
#16 0x00007f0e7902211d in aws_task_run (task=0x55e5176faf38, status=AWS_TASK_STATUS_RUN_READY) at external/aws-c-common/source/task_scheduler.c:44
#17 0x00007f0e79022801 in s_run_all (scheduler=0x7f0e701af6e0, current_time=20693612160159, status=AWS_TASK_STATUS_RUN_READY) at external/aws-c-common/source/task_scheduler.c:249
#18 0x00007f0e79022624 in aws_task_scheduler_run_all (scheduler=0x7f0e701af6e0, current_time=20693612160159) at external/aws-c-common/source/task_scheduler.c:188
#19 0x00007f0e78f4a9e3 in aws_event_loop_thread (args=0x7f0e701280b0) at external/aws-c-io/source/linux/epoll_event_loop.c:646
#20 0x00007f0e7901df40 in thread_fn (arg=0x7f0e701ce7d0) at external/aws-c-common/source/posix/thread.c:172
---Type <return> to continue, or q <return> to quit---
#21 0x00007f0e7c9a36db in start_thread (arg=0x7f0e477fe700) at pthread_create.c:463
#22 0x00007f0e7ccdc61f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Analysis

The GetObjectAsync call completes in frame 8, when GetObjectRequestShutdownCallback calls the GetObjectResponseReceivedHandler. This is also the time at which GetObject() releases the wait semaphore.

Some time after the download completed, the ResponseStream destructor is called in frame 2, which invokes ReleaseStream in frame 1 with m_underlyingStream->flush() on a rdbuf() that has already gone out of scope:

// source/utils/stream/ResponseStream.cpp
ResponseStream::~ResponseStream()
{
    ReleaseStream();
}

void ResponseStream::ReleaseStream()
{
    if (m_underlyingStream)
    {
        m_underlyingStream->flush();       // <=== HERE
        Aws::Delete(m_underlyingStream);
    }

    m_underlyingStream = nullptr;
}

What to do

Flushing the output may be useful when writing to disk (fstream). However, the placement of the flush() statement is problematic, since it happens after the download completed. In #58, it was already concluded that flush() should not happen before deletion, however the associated commit did not touch ResponseStream.cpp.

In conclusion, the flush() in ~ResponseStream should not happen, since it can not be guaranteed that the rdbuf() of the iostream is still within scope. There may be other places where such a flush() is better positioned. A fix is in #2320.

AWS CPP SDK version used

1.10.18 (problem also present on master).

Compiler and Version used

gcc / clang

Operating System and version

Linux, ubuntu 18.04

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.