Skip to content

Commit 561936f

Browse files
committed
More code changes to prevent use-after-free
1 parent 3a7dd32 commit 561936f

3 files changed

Lines changed: 22 additions & 17 deletions

File tree

google/cloud/storage/internal/bucket_metadata_cache.h

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "absl/types/optional.h"
2020
#include <cstddef>
2121
#include <list>
22+
#include <memory>
2223
#include <mutex>
2324
#include <string>
2425
#include <unordered_map>
@@ -68,15 +69,23 @@ class BucketMetadataCache {
6869

6970
class ScopedFetch {
7071
public:
71-
ScopedFetch(BucketMetadataCache* cache, std::string bucket_name)
72-
: cache_(cache), bucket_name_(std::move(bucket_name)) {}
73-
~ScopedFetch() {
74-
if (cache_) cache_->EndFetch(bucket_name_);
75-
}
72+
ScopedFetch() = default;
73+
ScopedFetch(std::shared_ptr<BucketMetadataCache> cache,
74+
std::string bucket_name)
75+
: state_(std::make_shared<State>(std::move(cache),
76+
std::move(bucket_name))) {}
7677

7778
private:
78-
BucketMetadataCache* cache_;
79-
std::string bucket_name_;
79+
struct State {
80+
State(std::shared_ptr<BucketMetadataCache> c, std::string b)
81+
: cache(std::move(c)), bucket_name(std::move(b)) {}
82+
~State() {
83+
if (cache) cache->EndFetch(bucket_name);
84+
}
85+
std::shared_ptr<BucketMetadataCache> cache;
86+
std::string bucket_name;
87+
};
88+
std::shared_ptr<State> state_;
8089
};
8190

8291
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END

google/cloud/storage/internal/tracing_connection.cc

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
3131
TracingConnection::TracingConnection(std::shared_ptr<StorageConnection> impl,
3232
AsyncRunner runner)
3333
: impl_(std::move(impl)),
34-
runner_(std::move(runner)),
35-
cache_(std::make_shared<BucketMetadataCache>(10000)) {}
34+
cache_(std::make_shared<BucketMetadataCache>()),
35+
runner_(std::move(runner)) {}
3636

3737
TracingConnection::~TracingConnection() = default;
3838

@@ -53,8 +53,6 @@ TracingConnection::AsyncRunner const& TracingConnection::runner() {
5353

5454
BucketMetadataCache& TracingConnection::cache() const { return *cache_; }
5555

56-
void TracingConnection::ResetCacheForTesting() { cache().Clear(); }
57-
5856
Options TracingConnection::options() const { return impl_->options(); }
5957

6058
void TracingConnection::EnrichSpan(opentelemetry::trace::Span& span,
@@ -69,10 +67,10 @@ void TracingConnection::MaybeTriggerBackgroundFetch(
6967
return;
7068
}
7169

70+
auto guard = ScopedFetch(cache_, bucket_name);
7271
auto current_options = google::cloud::internal::SaveCurrentOptions();
73-
runner()([impl = impl_, cache = cache_, bucket_name, current_options]() {
74-
ScopedFetch guard(cache.get(), bucket_name);
75-
72+
runner()([impl = impl_, cache = cache_, bucket_name, current_options,
73+
guard]() {
7674
google::cloud::internal::OptionsSpan span(current_options);
7775
storage::internal::GetBucketMetadataRequest request(bucket_name);
7876
auto result = impl->GetBucketMetadata(request);

google/cloud/storage/internal/tracing_connection.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@ class TracingConnection : public storage::internal::StorageConnection {
3636
AsyncRunner runner = {});
3737
~TracingConnection() override;
3838

39-
void ResetCacheForTesting();
40-
4139
Options options() const override;
4240

4341
StatusOr<storage::internal::ListBucketsResponse> ListBuckets(
@@ -209,9 +207,9 @@ class TracingConnection : public storage::internal::StorageConnection {
209207
AsyncRunner const& runner();
210208

211209
std::shared_ptr<StorageConnection> impl_;
212-
AsyncRunner runner_;
213210
std::shared_ptr<BucketMetadataCache> cache_;
214211
absl::once_flag once_flag_;
212+
AsyncRunner runner_;
215213
};
216214

217215
std::shared_ptr<storage::internal::StorageConnection> MakeTracingClient(

0 commit comments

Comments
 (0)