Skip to content

Commit 3c1f49a

Browse files
committed
fix: avoid lifetime extension to prevent holding lock (#79)
* fix: avoid lifetime extension to prevent holding lock * checkers
1 parent ce009e9 commit 3c1f49a

File tree

1 file changed

+18
-7
lines changed

1 file changed

+18
-7
lines changed

google/cloud/storage/internal/async/object_descriptor_impl.cc

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,14 @@ void ObjectDescriptorImpl::Flush(std::unique_lock<std::mutex> lk) {
7676
write_pending_ = true;
7777
google::storage::v2::BidiReadObjectRequest request;
7878
request.Swap(&next_request_);
79-
CurrentStream(std::move(lk))
80-
->Write(std::move(request))
81-
.then([w = WeakFromThis()](auto f) {
82-
if (auto self = w.lock()) self->OnWrite(f.get());
83-
});
79+
80+
// Assign CurrentStream to a temporary variable to prevent
81+
// lifetime extension which can cause the lock to be held until the
82+
// end of the block.
83+
auto current_stream = CurrentStream(std::move(lk));
84+
current_stream->Write(std::move(request)).then([w = WeakFromThis()](auto f) {
85+
if (auto self = w.lock()) self->OnWrite(f.get());
86+
});
8487
}
8588

8689
void ObjectDescriptorImpl::OnWrite(bool ok) {
@@ -91,7 +94,11 @@ void ObjectDescriptorImpl::OnWrite(bool ok) {
9194
}
9295

9396
void ObjectDescriptorImpl::DoRead(std::unique_lock<std::mutex> lk) {
94-
CurrentStream(std::move(lk))->Read().then([w = WeakFromThis()](auto f) {
97+
// Assign CurrentStream to a temporary variable to prevent
98+
// lifetime extension which can cause the lock to be held until the
99+
// end of the block.
100+
auto current_stream = CurrentStream(std::move(lk));
101+
current_stream->Read().then([w = WeakFromThis()](auto f) {
95102
if (auto self = w.lock()) self->OnRead(f.get());
96103
});
97104
}
@@ -136,7 +143,11 @@ void ObjectDescriptorImpl::CleanupDoneRanges(
136143
}
137144

138145
void ObjectDescriptorImpl::DoFinish(std::unique_lock<std::mutex> lk) {
139-
auto pending = CurrentStream(std::move(lk))->Finish();
146+
// Assign CurrentStream to a temporary variable to prevent
147+
// lifetime extension which can cause the lock to be held until the
148+
// end of the block.
149+
auto current_stream = CurrentStream(std::move(lk));
150+
auto pending = current_stream->Finish();
140151
if (!pending.valid()) return;
141152
pending.then([w = WeakFromThis()](auto f) {
142153
if (auto self = w.lock()) self->OnFinish(f.get());

0 commit comments

Comments
 (0)