From 60ab73cd1e616dcfd3a83d4d7dca01c045b464bc Mon Sep 17 00:00:00 2001 From: Lukas Tenbrink Date: Wed, 26 Nov 2025 20:20:17 +0100 Subject: [PATCH] Make `NodePath` copy-on-write, to avoid it changing accidentally by owned copies. --- core/string/node_path.cpp | 25 +++++++++++++++++++++++++ core/string/node_path.h | 8 ++++++++ 2 files changed, 33 insertions(+) diff --git a/core/string/node_path.cpp b/core/string/node_path.cpp index 8e454bdea8..2b6037ad63 100644 --- a/core/string/node_path.cpp +++ b/core/string/node_path.cpp @@ -42,6 +42,7 @@ void NodePath::_update_hash_cache() const { void NodePath::prepend_period() { if (data->path.size() && data->path[0].operator String() != ".") { + _copy_on_write(); data->path.insert(0, "."); data->concatenated_path = StringName(); data->hash_cache_valid = false; @@ -167,6 +168,29 @@ void NodePath::operator=(const NodePath &p_path) { } } +void NodePath::_copy_on_write() { + if (!data) { + return; // No data; nothing to do. + } + + if (data->refcount.get() == 1) { + return; // Already the only owner of data. + } + + // Make a copy. + Data *new_data = memnew(Data); + new_data->refcount.init(); + new_data->path = data->path; + new_data->subpath = data->subpath; + new_data->concatenated_path = data->concatenated_path; + new_data->concatenated_subpath = data->concatenated_subpath; + new_data->absolute = data->absolute; + new_data->hash_cache_valid = data->hash_cache_valid; + new_data->hash_cache = data->hash_cache; + unref(); + data = new_data; +} + NodePath::operator String() const { if (!data) { return String(); @@ -332,6 +356,7 @@ void NodePath::simplify() { if (!data) { return; } + _copy_on_write(); for (int i = 0; i < data->path.size(); i++) { if (data->path.size() == 1) { break; diff --git a/core/string/node_path.h b/core/string/node_path.h index 9ba10d4451..4636975c72 100644 --- a/core/string/node_path.h +++ b/core/string/node_path.h @@ -35,6 +35,10 @@ #include +// Represents a path to a node or property in a hierarchy of nodes +// Note that NodePath is (effectively) const: If you hold a NodePath, +// you can expect it to remain unchanged, even if you make copies of +// it. This is achieved through copy-on-write (CoW). class [[nodiscard]] NodePath { struct Data { SafeRefCount refcount; @@ -52,6 +56,10 @@ class [[nodiscard]] NodePath { void _update_hash_cache() const; + // Copies the underlying data. + // Every non-const function must call this before starting to mutate data. + void _copy_on_write(); + public: bool is_absolute() const; int get_name_count() const;