Skip to content

Commit b296898

Browse files
dconeybedconeybe
authored
Firestore: Add NoDestructor to suppress destructors on exit (#9885)
1 parent b6acec9 commit b296898

File tree

15 files changed

+217
-24
lines changed

15 files changed

+217
-24
lines changed

‎Firestore/core/src/bundle/bundle_serializer.cc

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include "Firestore/core/src/nanopb/message.h"
3535
#include "Firestore/core/src/nanopb/nanopb_util.h"
3636
#include "Firestore/core/src/timestamp_internal.h"
37+
#include "Firestore/core/src/util/no_destructor.h"
3738
#include "Firestore/core/src/util/statusor.h"
3839
#include "Firestore/core/src/util/string_format.h"
3940
#include "Firestore/core/src/util/string_util.h"
@@ -72,18 +73,19 @@ using nanopb::Message;
7273
using nanopb::SetRepeatedField;
7374
using nanopb::SharedMessage;
7475
using nlohmann::json;
76+
using util::NoDestructor;
7577
using util::StatusOr;
7678
using util::StringFormat;
7779
using Operator = FieldFilter::Operator;
7880

7981
namespace {
80-
const Bound kDefaultBound = Bound::FromValue(
81-
MakeSharedMessage<google_firestore_v1_ArrayValue>({}), false);
82+
const NoDestructor<Bound> kDefaultBound{Bound::FromValue(
83+
MakeSharedMessage<google_firestore_v1_ArrayValue>({}), false)};
8284
} // namespace
8385

8486
template <typename T>
8587
const std::vector<T>& EmptyVector() {
86-
static auto* empty = new std::vector<T>;
88+
static NoDestructor<std::vector<T>> empty;
8789
return *empty;
8890
}
8991

@@ -658,7 +660,7 @@ FilterList BundleSerializer::DecodeCompositeFilter(JsonReader& reader,
658660
Bound BundleSerializer::DecodeStartAtBound(JsonReader& reader,
659661
const json& query) const {
660662
if (!query.contains("startAt")) {
661-
return kDefaultBound;
663+
return *kDefaultBound;
662664
}
663665

664666
auto result =
@@ -669,7 +671,7 @@ Bound BundleSerializer::DecodeStartAtBound(JsonReader& reader,
669671
Bound BundleSerializer::DecodeEndAtBound(JsonReader& reader,
670672
const json& query) const {
671673
if (!query.contains("endAt")) {
672-
return kDefaultBound;
674+
return *kDefaultBound;
673675
}
674676

675677
auto result =

‎Firestore/core/src/core/direction.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#include <iosfwd>
2121
#include <string>
22+
#include <type_traits>
2223

2324
#include "Firestore/core/src/util/comparison.h"
2425
#include "absl/base/attributes.h"
@@ -67,6 +68,12 @@ class Direction {
6768
int comparison_modifier_ = AscendingModifier;
6869
};
6970

71+
static_assert(
72+
std::is_trivially_destructible<Direction>::value,
73+
"Direction should be trivially destructible so that Direction::Ascending "
74+
"and Direction::Descending are safe global constants; change to use "
75+
"NoDestructor if Direction is not trivially destructible.");
76+
7077
std::ostream& operator<<(std::ostream& os, const Direction& direction);
7178

7279
inline bool operator==(const Direction& lhs, const Direction& rhs) {

‎Firestore/core/src/credentials/auth_token.cc

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,14 @@
1919
#include <utility>
2020

2121
#include "Firestore/core/src/util/hard_assert.h"
22+
#include "Firestore/core/src/util/no_destructor.h"
2223

2324
namespace firebase {
2425
namespace firestore {
2526
namespace credentials {
2627

28+
using util::NoDestructor;
29+
2730
AuthToken::AuthToken() : token_{}, user_{User::Unauthenticated()} {
2831
}
2932

@@ -37,8 +40,8 @@ const std::string& AuthToken::token() const {
3740
}
3841

3942
const AuthToken& AuthToken::Unauthenticated() {
40-
static const AuthToken kUnauthenticatedToken{};
41-
return kUnauthenticatedToken;
43+
static const NoDestructor<AuthToken> kUnauthenticatedToken;
44+
return *kUnauthenticatedToken;
4245
}
4346

4447
} // namespace credentials

‎Firestore/core/src/credentials/user.cc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,14 @@
1919
#include <utility>
2020

2121
#include "Firestore/core/src/util/hard_assert.h"
22+
#include "Firestore/core/src/util/no_destructor.h"
2223

2324
namespace firebase {
2425
namespace firestore {
2526
namespace credentials {
2627

28+
using util::NoDestructor;
29+
2730
User::User() : is_authenticated_{false} {
2831
}
2932

@@ -32,7 +35,7 @@ User::User(std::string uid) : uid_{std::move(uid)}, is_authenticated_{true} {
3235
}
3336

3437
const User& User::Unauthenticated() {
35-
static const User* kUnauthenticated = new User();
38+
static const NoDestructor<User> kUnauthenticated;
3639
return *kUnauthenticated;
3740
}
3841

‎Firestore/core/src/local/leveldb_transaction.cc

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
* limitations under the License.
1515
*/
1616

17+
#include <type_traits>
18+
1719
#include "Firestore/core/src/local/leveldb_transaction.h"
1820

1921
#include "Firestore/core/src/local/leveldb_key.h"
@@ -146,7 +148,9 @@ LevelDbTransaction::LevelDbTransaction(DB* db,
146148
}
147149

148150
const ReadOptions& LevelDbTransaction::DefaultReadOptions() {
149-
// ReadOptions is trivial so it does not need to be heap-allocated.
151+
static_assert(std::is_trivially_destructible<ReadOptions>::value,
152+
"ReadOptions should be trivially-destructible; otherwise, it "
153+
"should use NoDestructor below.");
150154
static ReadOptions options = [] {
151155
ReadOptions read_options;
152156
read_options.verify_checksums = true;
@@ -156,7 +160,9 @@ const ReadOptions& LevelDbTransaction::DefaultReadOptions() {
156160
}
157161

158162
const WriteOptions& LevelDbTransaction::DefaultWriteOptions() {
159-
// WriteOptions is trivial so it does not need to be heap-allocated.
163+
static_assert(std::is_trivially_destructible<WriteOptions>::value,
164+
"WriteOptions should be trivially-destructible; otherwise, it "
165+
"should use NoDestructor below.");
160166
static WriteOptions options;
161167
return options;
162168
}

‎Firestore/core/src/model/field_path.cc

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
#include "Firestore/core/src/util/exception.h"
2323
#include "Firestore/core/src/util/hard_assert.h"
24+
#include "Firestore/core/src/util/no_destructor.h"
2425
#include "Firestore/core/src/util/status.h"
2526
#include "Firestore/core/src/util/statusor.h"
2627
#include "absl/strings/str_join.h"
@@ -32,6 +33,7 @@ namespace firestore {
3233
namespace model {
3334
namespace {
3435

36+
using util::NoDestructor;
3537
using util::Status;
3638
using util::StatusOr;
3739
using util::StringFormat;
@@ -198,13 +200,14 @@ StatusOr<FieldPath> FieldPath::FromServerFormatView(absl::string_view path) {
198200
}
199201

200202
const FieldPath& FieldPath::EmptyPath() {
201-
static const FieldPath empty_path;
202-
return empty_path;
203+
static const NoDestructor<FieldPath> empty_path;
204+
return *empty_path;
203205
}
204206

205207
const FieldPath& FieldPath::KeyFieldPath() {
206-
static const FieldPath key_field_path{FieldPath::kDocumentKeyPath};
207-
return key_field_path;
208+
static const NoDestructor<FieldPath> key_field_path(
209+
FieldPath{FieldPath::kDocumentKeyPath});
210+
return *key_field_path;
208211
}
209212

210213
bool FieldPath::IsKeyFieldPath() const {

‎Firestore/core/src/model/snapshot_version.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "Firestore/core/src/model/snapshot_version.h"
1818

1919
#include <ostream>
20+
#include <type_traits>
2021

2122
#include "Firestore/core/src/util/hashing.h"
2223

@@ -29,6 +30,9 @@ SnapshotVersion::SnapshotVersion(const Timestamp& timestamp)
2930
}
3031

3132
const SnapshotVersion& SnapshotVersion::None() {
33+
static_assert(std::is_trivially_destructible<SnapshotVersion>::value,
34+
"SnapshotVersion should be trivially-destructible; otherwise, "
35+
"it should use NoDestructor below.");
3236
static const SnapshotVersion kNone(Timestamp{});
3337
return kNone;
3438
}

‎Firestore/core/src/model/value_util.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <cmath>
2121
#include <limits>
2222
#include <memory>
23+
#include <type_traits>
2324
#include <utility>
2425
#include <vector>
2526

@@ -696,6 +697,11 @@ google_firestore_v1_Value MaxValue() {
696697
// Make `field_entry` static so that it has a memory address that outlives
697698
// this function's scope; otherwise, using its address in the `map_value`
698699
// variable below would be invalid by the time the caller accessed it.
700+
static_assert(
701+
std::is_trivially_destructible<
702+
google_firestore_v1_MapValue_FieldsEntry>::value,
703+
"google_firestore_v1_MapValue_FieldsEntry should be "
704+
"trivially-destructible; otherwise, it should use NoDestructor below.");
699705
static google_firestore_v1_MapValue_FieldsEntry field_entry;
700706
field_entry.key = kMaxValueFieldKey;
701707
field_entry.value = value;

‎Firestore/core/src/remote/datastore.cc

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#include "Firestore/core/src/util/executor.h"
4141
#include "Firestore/core/src/util/hard_assert.h"
4242
#include "Firestore/core/src/util/log.h"
43+
#include "Firestore/core/src/util/no_destructor.h"
4344
#include "Firestore/core/src/util/statusor.h"
4445
#include "absl/memory/memory.h"
4546
#include "absl/strings/str_cat.h"
@@ -367,9 +368,10 @@ bool Datastore::IsPermanentWriteError(const Status& error) {
367368

368369
std::string Datastore::GetAllowlistedHeadersAsString(
369370
const GrpcCall::Metadata& headers) {
370-
static auto* allowlist = new std::unordered_set<std::string>{
371-
"date", "x-google-backends", "x-google-netmon-label", "x-google-service",
372-
"x-google-gfe-request-trace"};
371+
static const util::NoDestructor<std::unordered_set<std::string>> allowlist(
372+
std::unordered_set<std::string>{
373+
"date", "x-google-backends", "x-google-netmon-label",
374+
"x-google-service", "x-google-gfe-request-trace"});
373375

374376
std::string result;
375377
auto end = allowlist->end();

‎Firestore/core/src/remote/grpc_connection.cc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include "Firestore/core/src/util/filesystem.h"
3333
#include "Firestore/core/src/util/hard_assert.h"
3434
#include "Firestore/core/src/util/log.h"
35+
#include "Firestore/core/src/util/no_destructor.h"
3536
#include "Firestore/core/src/util/statusor.h"
3637
#include "Firestore/core/src/util/string_format.h"
3738
#include "Firestore/core/src/util/warnings.h"
@@ -157,7 +158,7 @@ class HostConfigMap {
157158
};
158159

159160
HostConfigMap& Config() {
160-
static auto* config_by_host = new HostConfigMap();
161+
static util::NoDestructor<HostConfigMap> config_by_host;
161162
return *config_by_host;
162163
}
163164

@@ -202,7 +203,7 @@ class ClientLanguageToken {
202203
};
203204

204205
ClientLanguageToken& LanguageToken() {
205-
static auto* token = new ClientLanguageToken();
206+
static util::NoDestructor<ClientLanguageToken> token;
206207
return *token;
207208
}
208209

0 commit comments

Comments
 (0)