Skip to content

Commit

Permalink
fix(container): fix possible init order issues with Vector_Instrument…
Browse files Browse the repository at this point in the history
…ation

See inline comment in Vector_Instrumentation::instance.
  • Loading branch information
strager committed Nov 7, 2023
1 parent a79dcc5 commit 3ed1c3f
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 18 deletions.
8 changes: 6 additions & 2 deletions src/quick-lint-js/container/vector-profiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@ QLJS_WARNING_IGNORE_MSVC(4996) // Function or variable may be unsafe.
namespace quick_lint_js {
#if QLJS_FEATURE_VECTOR_PROFILING
Vector_Instrumentation &Vector_Instrumentation::instance() {
static Vector_Instrumentation instrumentation;
return instrumentation;
// NOTE(strager): We intentionally leak the Vector_Instrumentation. We want to
// avoid deinitialization order issues, so defer deinitialization forever.
// Also, cleaning up the memory is wasteful and is unnecessary for modern leak
// checkers anyway.
static Vector_Instrumentation *instrumentation = new Vector_Instrumentation();
return *instrumentation;
}
#endif

Expand Down
2 changes: 1 addition & 1 deletion src/quick-lint-js/debug/debug-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ std::shared_ptr<Debug_Server> Debug_Server::create() {
return instance;
}

Raw_Vector<std::shared_ptr<Debug_Server>> Debug_Server::instances() {
Vector<std::shared_ptr<Debug_Server>> Debug_Server::instances() {
return Instance_Tracker<Debug_Server>::instances();
}

Expand Down
2 changes: 1 addition & 1 deletion src/quick-lint-js/debug/debug-server.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class Debug_Server {

public:
static std::shared_ptr<Debug_Server> create();
static Raw_Vector<std::shared_ptr<Debug_Server>> instances();
static Vector<std::shared_ptr<Debug_Server>> instances();

explicit Debug_Server(Create_Tag);

Expand Down
5 changes: 1 addition & 4 deletions src/quick-lint-js/logging/logger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,7 @@ QLJS_WARNING_IGNORE_GCC("-Wsuggest-attribute=format")
namespace quick_lint_js {
namespace {
struct Global_Loggers {
// NOTE(strager): We use Raw_Vector here instead of Vector. Otherwise, in
// vector instrumented builds, loggers might be initialized before the vector
// profiler is initialized, causing use-before-init issues.
Raw_Vector<Logger*> loggers{new_delete_resource()};
Vector<Logger*> loggers{"Global_Loggers::loggers", new_delete_resource()};
bool initialized = false;

void initialize_if_needed() {
Expand Down
18 changes: 8 additions & 10 deletions src/quick-lint-js/util/instance-tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,17 @@ template <class Tracked>
class Instance_Tracker {
public:
static void track(std::shared_ptr<Tracked> instance) {
Lock_Ptr<Raw_Vector<std::weak_ptr<Tracked>>> weak_instances =
Lock_Ptr<Vector<std::weak_ptr<Tracked>>> weak_instances =
weak_instances_.lock();
sanitize_instances(weak_instances);
weak_instances->push_back(std::move(instance));
}

static Raw_Vector<std::shared_ptr<Tracked>> instances() {
Raw_Vector<std::shared_ptr<Tracked>> instances(new_delete_resource());
static Vector<std::shared_ptr<Tracked>> instances() {
Vector<std::shared_ptr<Tracked>> instances("instances",
new_delete_resource());
{
Lock_Ptr<Raw_Vector<std::weak_ptr<Tracked>>> weak_instances =
Lock_Ptr<Vector<std::weak_ptr<Tracked>>> weak_instances =
weak_instances_.lock();
sanitize_instances(weak_instances);
instances.reserve(weak_instances->size());
Expand All @@ -45,7 +46,7 @@ class Instance_Tracker {

private:
static void sanitize_instances(
Lock_Ptr<Raw_Vector<std::weak_ptr<Tracked>>>& weak_instances) {
Lock_Ptr<Vector<std::weak_ptr<Tracked>>>& weak_instances) {
erase_if(*weak_instances, [](const std::weak_ptr<Tracked>& weak_instance) {
return weak_instance.expired();
});
Expand All @@ -55,11 +56,8 @@ class Instance_Tracker {
sanitize_instances(weak_instances_.lock());
}

// NOTE(strager): We use Raw_Vector here instead of Vector. Otherwise, in
// vector instrumented builds, weak_instances_ might be initialized before the
// vector profiler is initialized, causing use-before-init issues.
static inline Synchronized<Raw_Vector<std::weak_ptr<Tracked>>>
weak_instances_{new_delete_resource()};
static inline Synchronized<Vector<std::weak_ptr<Tracked>>> weak_instances_{
"weak_instances_", new_delete_resource()};
};
}

Expand Down

0 comments on commit 3ed1c3f

Please sign in to comment.