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.

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

impl(rest): set ReadFunctionAbort via RAII #14617

Merged
merged 3 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 22 additions & 12 deletions google/cloud/internal/curl_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,22 @@ std::size_t CurlImpl::HeaderCallback(absl::Span<char> response) {
response.size());
}

class CurlImpl::ReadFunctionAbortGuard {
public:
explicit ReadFunctionAbortGuard(CurlImpl& impl) : impl_(impl) {}
~ReadFunctionAbortGuard() {
// If curl_closed_ is true, then the handle has already been recycled and
// attempting to set an option on it will error.
if (!impl_.curl_closed_) {
impl_.handle_.SetOptionUnchecked(CURLOPT_READFUNCTION,
&ReadFunctionAbort);
}
}

private:
CurlImpl& impl_;
};

Status CurlImpl::MakeRequestImpl(RestContext& context) {
TRACE_STATE() << ", url_=" << url_;

Expand All @@ -509,6 +525,11 @@ Status CurlImpl::MakeRequestImpl(RestContext& context) {
handle_.SetOptionUnchecked(CURLOPT_HTTP_VERSION,
VersionToCurlCode(http_version_));

// All data in the WriteVector should be written after ReadImpl returns unless
// an error, typically a timeout, has occurred. Use ReadFunctionAbortGuard to
// leverage RAII to instruct curl to not attempt to send anymore data on this
// handle regardless if an error or exception is encountered.
ReadFunctionAbortGuard guard(*this);
auto error = curl_multi_add_handle(multi_.get(), handle_.handle_.get());

// This indicates that we are using the API incorrectly. The application
Expand All @@ -524,18 +545,7 @@ Status CurlImpl::MakeRequestImpl(RestContext& context) {
// thus make available the status_code and headers. Any response data
// should be put into the spill buffer, which makes them available for
// subsequent calls to Read() after the headers have been extracted.
auto read_status = ReadImpl(context, {}).status();

// All data in the WriteVector should have been written by now, unless an
// error, typically a timeout, has occurred. Instruct curl to not attempt to
// send anymore data on this handle. If curl_closed_ is true, then the handle
// has already been recycled and attempting to set an option on it will error.
if (!curl_closed_) {
status = handle_.SetOption(CURLOPT_READFUNCTION, &ReadFunctionAbort);
if (!status.ok()) return OnTransferError(context, std::move(status));
}

return read_status;
return ReadImpl(context, {}).status();
}

StatusOr<std::size_t> CurlImpl::ReadImpl(RestContext& context,
Expand Down
1 change: 1 addition & 0 deletions google/cloud/internal/curl_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ class CurlImpl {
std::size_t HeaderCallback(absl::Span<char> response);

private:
class ReadFunctionAbortGuard;
Status MakeRequestImpl(RestContext& context);
StatusOr<std::size_t> ReadImpl(RestContext& context, absl::Span<char> output);

Expand Down
Loading