Skip to content

Commit 5af6993

Browse files
apolyakovapolyakov
authored
[k2] fix reference counter for constants (#1484)
Certain KPHP types, such as array, string, and class_instance, utilize an internal copy-on-write mechanism with a reference counter. To ensure thread safety, all constants of these types must have their reference counter set to ExtraRefCnt::for_global_const. While the compiler typically generates the necessary code to set this reference counter during constant initialization, there are scenarios where it fails to do so. Consider the following example: $cities = [['Japan' => 'Tokyo'], ['US' => 'San Francisco'], ['Russia' => 'Saint Petersburg'], ['Moldova' => 'Tiraspol']]; // array<array<string>> $avg_apartment_prices = [1800, 3500, 500, 200]; // array<int> $cities_with_prices = [$cities, $avg_apartment_prices]; // array<mixed> When initializing the constant for $cities_with_prices, the $cities array needs to be converted from array<array<string>> to mixed. This conversion involves an implicit transformation of the inner array<string> to array<mixed>, resulting in the creation of a new array object. Since the compiler is unaware of this newly created object, it does not set its reference counter to the special ExtraRefCnt::for_global_const value. Consequently, each access to the implicitly created array triggers an increment and decrement of its reference counter, leading to a race condition. This race condition can easily result in a segmentation fault at runtime. Solution: To resolve this issue, we have implemented two functions: kphp::core::set_reference_counter_recursive and kphp::core::is_reference_counter_recursive. kphp::core::set_reference_counter_recursive: This function is invoked for each constant that the compiler is aware of. It recursively traverses the structure of the constant, including any implicitly created objects, and sets their reference counters to ExtraRefCnt::for_global_const. By doing so, it ensures that all reference-counted objects, even those created implicitly during type conversions, have their reference counters initialized to a thread-safe value. kphp::core::is_reference_counter_recursive: This function is used to verify that the reference counters of all objects within a constant are correctly set to ExtraRefCnt::for_global_const. It provides a mechanism to ensure that the initialization process has been correctly applied, offering an additional layer of validation. Future work: support tuples, shapes, and class instance's members in kphp::core::set_reference_counter_recursive and kphp::core::is_reference_counter_recursive.
1 parent 0684844 commit 5af6993

File tree

8 files changed

+233
-43
lines changed

8 files changed

+233
-43
lines changed

‎compiler/code-gen/files/const-vars-init.cpp‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,4 +168,84 @@ void ConstVarsInit::compile(CodeGenerator &W) const {
168168
W << ExternInclude(G->settings().runtime_headers.get());
169169
compile_const_init(W, all_constants_in_mem);
170170
W << CloseFile();
171+
172+
if (G->is_output_mode_k2()) {
173+
// generate setters
174+
for (const auto& batch : all_constants_in_mem.get_batches()) {
175+
W << OpenFile("c." + std::to_string(batch.batch_idx) + ".cpp", "o_const_vars_set_reference_counter", false);
176+
W << ExternInclude(G->settings().runtime_headers.get());
177+
178+
IncludesCollector includes;
179+
ConstantsExternCollector c_mem_extern;
180+
for (VarPtr var : batch.constants) {
181+
if (!G->is_output_mode_lib() && !G->is_output_mode_k2_lib()) {
182+
includes.add_var_signature_depends(var);
183+
includes.add_vertex_depends(var->init_val);
184+
}
185+
}
186+
W << includes;
187+
188+
for (VarPtr var : batch.constants) {
189+
W << "extern " << type_out(tinf::get_type(var)) << " " << var->name << ";" << NL;
190+
}
191+
192+
FunctionSignatureGenerator(W) << "void const_vars_set_reference_counter" << std::to_string(batch.batch_idx) << "()" << BEGIN;
193+
for (VarPtr var : batch.constants) {
194+
if (!(vk::any_of_equal(var->tinf_node.get_type()->ptype(), tp_string, tp_array, tp_Class, tp_mixed))) {
195+
continue;
196+
}
197+
W << "kphp::core::set_reference_counter_recursive(" << var->name << ", ExtraRefCnt::for_global_const);" << NL;
198+
}
199+
W << END;
200+
W << CloseFile();
201+
}
202+
203+
W << OpenFile("const_vars_set_reference_counter.cpp", "", false);
204+
FunctionSignatureGenerator(W) << "void const_vars_set_reference_counter()" << BEGIN;
205+
for (const auto& batch : all_constants_in_mem.get_batches()) {
206+
FunctionSignatureGenerator(W) << "void const_vars_set_reference_counter" << std::to_string(batch.batch_idx) << "()" << SemicolonAndNL();
207+
W << "const_vars_set_reference_counter" << std::to_string(batch.batch_idx) << "()" << SemicolonAndNL();
208+
}
209+
W << END;
210+
W << CloseFile();
211+
212+
// generate checkers
213+
for (const auto& batch : all_constants_in_mem.get_batches()) {
214+
W << OpenFile("c." + std::to_string(batch.batch_idx) + ".cpp", "o_const_vars_check_reference_counter", false);
215+
W << ExternInclude(G->settings().runtime_headers.get());
216+
217+
IncludesCollector includes;
218+
ConstantsExternCollector c_mem_extern;
219+
for (VarPtr var : batch.constants) {
220+
if (!G->is_output_mode_lib() && !G->is_output_mode_k2_lib()) {
221+
includes.add_var_signature_depends(var);
222+
includes.add_vertex_depends(var->init_val);
223+
}
224+
}
225+
W << includes;
226+
227+
for (VarPtr var : batch.constants) {
228+
W << "extern " << type_out(tinf::get_type(var)) << " " << var->name << ";" << NL;
229+
}
230+
231+
FunctionSignatureGenerator(W) << "void const_vars_check_reference_counter" << std::to_string(batch.batch_idx) << "()" << BEGIN;
232+
for (VarPtr var : batch.constants) {
233+
if (!(vk::any_of_equal(var->tinf_node.get_type()->ptype(), tp_string, tp_array, tp_Class, tp_mixed))) {
234+
continue;
235+
}
236+
W << "kphp::log::assertion(kphp::core::is_reference_counter_recursive(" << var->name << ", ExtraRefCnt::for_global_const));" << NL;
237+
}
238+
W << END;
239+
W << CloseFile();
240+
}
241+
242+
W << OpenFile("const_vars_check_reference_counter.cpp", "", false);
243+
FunctionSignatureGenerator(W) << "void const_vars_check_reference_counter()" << BEGIN;
244+
for (const auto& batch : all_constants_in_mem.get_batches()) {
245+
FunctionSignatureGenerator(W) << "void const_vars_check_reference_counter" << std::to_string(batch.batch_idx) << "()" << SemicolonAndNL();
246+
W << "const_vars_check_reference_counter" << std::to_string(batch.batch_idx) << "()" << SemicolonAndNL();
247+
}
248+
W << END;
249+
W << CloseFile();
250+
}
171251
}

‎compiler/code-gen/files/init-scripts.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,15 @@ void StaticInit::compile(CodeGenerator &W) const {
8080
W << lib->lib_namespace() << "::const_vars_init();" << NL;
8181
}
8282
}
83+
84+
if (G->is_output_mode_k2()) {
85+
FunctionSignatureGenerator(W) << "void const_vars_set_reference_counter()" << SemicolonAndNL() << NL;
86+
W << "const_vars_set_reference_counter();" << NL;
87+
88+
FunctionSignatureGenerator(W) << "void const_vars_check_reference_counter()" << SemicolonAndNL() << NL;
89+
W << "const_vars_check_reference_counter();" << NL;
90+
}
91+
8392
W << NL;
8493
FunctionSignatureGenerator(W) << "void " << ShapeKeys::get_function_name() << "()" << SemicolonAndNL();
8594
W << ShapeKeys::get_function_name() << "();" << NL;

‎runtime-light/allocator/runtime-light-allocator.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
#include <cstring>
88

99
#include "runtime-light/allocator/allocator-state.h"
10-
#include "runtime-light/allocator/allocator.h"
1110
#include "runtime-light/k2-platform/k2-api.h"
1211
#include "runtime-light/stdlib/diagnostics/logs.h"
1312

‎runtime-light/core/reference-counter/reference-counter-functions.h‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
// Compiler for PHP (aka KPHP)
2+
// Copyright (c) 2025 LLC «V Kontakte»
3+
// Distributed under the GPL v3 License, see LICENSE.notice.txt
4+
5+
#pragma once
6+
7+
#include <utility>
8+
9+
#include "common/php-functions.h"
10+
#include "runtime-common/core/runtime-core.h"
11+
12+
namespace kphp::core {
13+
14+
template<typename T>
15+
void set_reference_counter_recursive(T& /*obj*/, ExtraRefCnt /*rc*/) noexcept {}
16+
17+
template<typename T>
18+
void set_reference_counter_recursive(Optional<T>& obj, ExtraRefCnt rc) noexcept {
19+
if (obj.has_value()) {
20+
set_reference_counter_recursive(obj.val(), rc);
21+
}
22+
}
23+
24+
template<typename T>
25+
void set_reference_counter_recursive(class_instance<T>& obj, ExtraRefCnt rc) noexcept {
26+
if (!obj.is_null()) {
27+
// TODO we need some visitor to visit all the class member
28+
obj.set_reference_counter_to(rc);
29+
}
30+
}
31+
32+
template<typename T>
33+
void set_reference_counter_recursive(array<T>& obj, ExtraRefCnt rc) noexcept {
34+
obj.set_reference_counter_to(rc);
35+
for (const auto& it : std::as_const(obj)) {
36+
auto key{it.get_key()};
37+
set_reference_counter_recursive(key, rc);
38+
auto value{it.get_value()};
39+
set_reference_counter_recursive(value, rc);
40+
}
41+
}
42+
43+
template<>
44+
inline void set_reference_counter_recursive<string>(string& obj, ExtraRefCnt rc) noexcept {
45+
return obj.set_reference_counter_to(rc);
46+
}
47+
48+
template<>
49+
inline void set_reference_counter_recursive<mixed>(mixed& obj, ExtraRefCnt rc) noexcept {
50+
switch (obj.get_type()) {
51+
case mixed::type::NUL:
52+
case mixed::type::BOOLEAN:
53+
case mixed::type::INTEGER:
54+
case mixed::type::FLOAT:
55+
return;
56+
case mixed::type::STRING:
57+
case mixed::type::OBJECT:
58+
return obj.set_reference_counter_to(rc);
59+
case mixed::type::ARRAY:
60+
return set_reference_counter_recursive(obj.as_array(), rc);
61+
}
62+
}
63+
64+
// ================================================================================================
65+
66+
template<typename T>
67+
bool is_reference_counter_recursive(const T& /*obj*/, ExtraRefCnt /*rc*/) noexcept {
68+
return true;
69+
}
70+
71+
template<typename T>
72+
bool is_reference_counter_recursive(const Optional<T>& obj, ExtraRefCnt rc) noexcept {
73+
return obj.has_value() ? is_reference_counter_recursive(obj.val(), rc) : true;
74+
}
75+
76+
template<typename T>
77+
bool is_reference_counter_recursive(const class_instance<T>& obj, ExtraRefCnt rc) noexcept {
78+
// TODO we need some visitor to visit all the class members
79+
return !obj.is_null() ? obj.is_reference_counter(rc) : true;
80+
}
81+
82+
template<typename T>
83+
bool is_reference_counter_recursive(const array<T>& obj, ExtraRefCnt rc) noexcept {
84+
if (!obj.is_reference_counter(rc)) {
85+
return false;
86+
}
87+
88+
for (const auto& it : obj) { // NOLINT
89+
if (!is_reference_counter_recursive(it.get_key(), rc) || !is_reference_counter_recursive(it.get_value(), rc)) {
90+
return false;
91+
}
92+
}
93+
return true;
94+
}
95+
96+
template<>
97+
inline bool is_reference_counter_recursive<string>(const string& obj, ExtraRefCnt rc) noexcept {
98+
return obj.is_reference_counter(rc);
99+
}
100+
101+
template<>
102+
inline bool is_reference_counter_recursive<mixed>(const mixed& obj, ExtraRefCnt rc) noexcept {
103+
switch (obj.get_type()) {
104+
case mixed::type::NUL:
105+
case mixed::type::BOOLEAN:
106+
case mixed::type::INTEGER:
107+
case mixed::type::FLOAT:
108+
return true;
109+
case mixed::type::STRING:
110+
case mixed::type::OBJECT:
111+
return obj.is_reference_counter(rc);
112+
case mixed::type::ARRAY:
113+
return is_reference_counter_recursive(obj.as_array(), rc);
114+
}
115+
}
116+
117+
} // namespace kphp::core

‎runtime-light/state/component-state.cpp‎

Lines changed: 1 addition & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -13,46 +13,18 @@
1313
#include <sys/stat.h>
1414
#include <utility>
1515

16-
#include "common/php-functions.h"
1716
#include "runtime-common/core/allocator/script-malloc-interface.h"
1817
#include "runtime-common/core/runtime-core.h"
1918
#include "runtime-common/stdlib/serialization/json-functions.h"
2019
#include "runtime-light/k2-platform/k2-api.h"
2120
#include "runtime-light/stdlib/diagnostics/logs.h"
2221
#include "runtime-light/stdlib/file/resource.h"
2322

24-
namespace {
25-
26-
void set_reference_counter_recursive(mixed obj, ExtraRefCnt rc) noexcept {
27-
switch (obj.get_type()) {
28-
case mixed::type::ARRAY:
29-
obj.set_reference_counter_to(rc);
30-
for (const auto& it : std::as_const(obj.as_array())) {
31-
set_reference_counter_recursive(it.get_key(), rc);
32-
set_reference_counter_recursive(it.get_value(), rc);
33-
}
34-
break;
35-
default:
36-
obj.set_reference_counter_to(rc);
37-
break;
38-
}
39-
}
40-
41-
} // namespace
42-
4323
void ComponentState::parse_env() noexcept {
4424
for (auto i = 0; i < envc; ++i) {
4525
const auto [env_key, env_value]{k2::env_fetch(i)};
46-
47-
string key_str{env_key.get()};
48-
key_str.set_reference_counter_to(ExtraRefCnt::for_global_const);
49-
50-
string value_str{env_value.get()};
51-
value_str.set_reference_counter_to(ExtraRefCnt::for_global_const);
52-
53-
env.set_value(key_str, value_str);
26+
env.set_value(string{env_key.get()}, string{env_value.get()});
5427
}
55-
env.set_reference_counter_to(ExtraRefCnt::for_global_const);
5628
}
5729

5830
void ComponentState::parse_ini_arg(std::string_view key_view, std::string_view value_view) noexcept {
@@ -61,11 +33,7 @@ void ComponentState::parse_ini_arg(std::string_view key_view, std::string_view v
6133
return;
6234
}
6335
string key_str{std::next(key_view.data(), INI_ARG_PREFIX.size()), static_cast<string::size_type>(key_view.size() - INI_ARG_PREFIX.size())};
64-
key_str.set_reference_counter_to(ExtraRefCnt::for_global_const);
65-
6636
string value_str{value_view.data(), static_cast<string::size_type>(value_view.size())};
67-
value_str.set_reference_counter_to(ExtraRefCnt::for_global_const);
68-
6937
ini_opts.set_value(key_str, value_str);
7038
}
7139

@@ -123,6 +91,4 @@ void ComponentState::parse_args() noexcept {
12391
kphp::log::warning("unexpected argument format: {}", key_view);
12492
}
12593
}
126-
ini_opts.set_reference_counter_to(ExtraRefCnt::for_global_const);
127-
set_reference_counter_recursive(runtime_config, ExtraRefCnt::for_global_const);
12894
}

‎runtime-light/state/component-state.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,12 @@
1010
#include <string_view>
1111

1212
#include "common/mixin/not_copyable.h"
13+
#include "common/php-functions.h"
1314
#include "runtime-common/core/runtime-core.h"
1415
#include "runtime-light/allocator/allocator-state.h"
16+
#include "runtime-light/core/reference-counter/reference-counter-functions.h"
1517
#include "runtime-light/k2-platform/k2-api.h"
18+
#include "runtime-light/stdlib/diagnostics/logs.h"
1619
#include "runtime-light/stdlib/kml/kml-state.h"
1720

1821
struct ComponentState final : private vk::not_copyable {
@@ -28,6 +31,13 @@ struct ComponentState final : private vk::not_copyable {
2831
ComponentState() noexcept {
2932
parse_env();
3033
parse_args();
34+
35+
kphp::log::assertion((kphp::core::set_reference_counter_recursive(ini_opts, ExtraRefCnt::for_global_const),
36+
kphp::core::is_reference_counter_recursive(ini_opts, ExtraRefCnt::for_global_const)));
37+
kphp::log::assertion((kphp::core::set_reference_counter_recursive(env, ExtraRefCnt::for_global_const),
38+
kphp::core::is_reference_counter_recursive(env, ExtraRefCnt::for_global_const)));
39+
kphp::log::assertion((kphp::core::set_reference_counter_recursive(runtime_config, ExtraRefCnt::for_global_const),
40+
kphp::core::is_reference_counter_recursive(runtime_config, ExtraRefCnt::for_global_const)));
3141
}
3242

3343
static const ComponentState& get() noexcept {

‎runtime-light/state/image-state.h‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "common/php-functions.h"
1717
#include "runtime-common/core/runtime-core.h"
1818
#include "runtime-light/allocator/allocator-state.h"
19+
#include "runtime-light/core/reference-counter/reference-counter-functions.h"
1920
#include "runtime-light/k2-platform/k2-api.h"
2021
#include "runtime-light/stdlib/diagnostics/logs.h"
2122
#include "runtime-light/stdlib/file/file-system-state.h"
@@ -72,12 +73,18 @@ struct ImageState final : private vk::not_copyable {
7273
uname_info_a.push_back(' ');
7374
uname_info_a.append(uname_info_m);
7475
// prevent race condition on reference counter
75-
uname_info_s.set_reference_counter_to(ExtraRefCnt::for_global_const);
76-
uname_info_n.set_reference_counter_to(ExtraRefCnt::for_global_const);
77-
uname_info_r.set_reference_counter_to(ExtraRefCnt::for_global_const);
78-
uname_info_v.set_reference_counter_to(ExtraRefCnt::for_global_const);
79-
uname_info_m.set_reference_counter_to(ExtraRefCnt::for_global_const);
80-
uname_info_a.set_reference_counter_to(ExtraRefCnt::for_global_const);
76+
kphp::log::assertion((kphp::core::set_reference_counter_recursive(uname_info_s, ExtraRefCnt::for_global_const),
77+
kphp::core::is_reference_counter_recursive(uname_info_s, ExtraRefCnt::for_global_const)));
78+
kphp::log::assertion((kphp::core::set_reference_counter_recursive(uname_info_n, ExtraRefCnt::for_global_const),
79+
kphp::core::is_reference_counter_recursive(uname_info_n, ExtraRefCnt::for_global_const)));
80+
kphp::log::assertion((kphp::core::set_reference_counter_recursive(uname_info_r, ExtraRefCnt::for_global_const),
81+
kphp::core::is_reference_counter_recursive(uname_info_r, ExtraRefCnt::for_global_const)));
82+
kphp::log::assertion((kphp::core::set_reference_counter_recursive(uname_info_v, ExtraRefCnt::for_global_const),
83+
kphp::core::is_reference_counter_recursive(uname_info_v, ExtraRefCnt::for_global_const)));
84+
kphp::log::assertion((kphp::core::set_reference_counter_recursive(uname_info_m, ExtraRefCnt::for_global_const),
85+
kphp::core::is_reference_counter_recursive(uname_info_m, ExtraRefCnt::for_global_const)));
86+
kphp::log::assertion((kphp::core::set_reference_counter_recursive(uname_info_a, ExtraRefCnt::for_global_const),
87+
kphp::core::is_reference_counter_recursive(uname_info_a, ExtraRefCnt::for_global_const)));
8188
}
8289

8390
static const ImageState& get() noexcept {

‎runtime-light/stdlib/rpc/rpc-tl-builtins.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "common/php-functions.h"
1212
#include "common/tl/constants/common.h"
1313
#include "runtime-common/core/runtime-core.h"
14+
#include "runtime-light/core/reference-counter/reference-counter-functions.h"
1415
#include "runtime-light/server/rpc/rpc-server-state.h"
1516
#include "runtime-light/stdlib/diagnostics/exception-functions.h"
1617
#include "runtime-light/stdlib/diagnostics/logs.h"
@@ -22,7 +23,8 @@
2223
inline void register_tl_storers_table_and_fetcher(const array<tl_storer_ptr>& gen$ht, tl_fetch_wrapper_ptr gen$t_ReqResult_fetch) noexcept {
2324
auto& rpc_mutable_image_state{RpcImageState::get_mutable()};
2425
rpc_mutable_image_state.tl_storers_ht = gen$ht;
25-
rpc_mutable_image_state.tl_storers_ht.set_reference_counter_to(ExtraRefCnt::for_global_const);
26+
kphp::log::assertion((kphp::core::set_reference_counter_recursive(rpc_mutable_image_state.tl_storers_ht, ExtraRefCnt::for_global_const),
27+
kphp::core::is_reference_counter_recursive(rpc_mutable_image_state.tl_storers_ht, ExtraRefCnt::for_global_const)));
2628
rpc_mutable_image_state.tl_fetch_wrapper = gen$t_ReqResult_fetch;
2729
}
2830

0 commit comments

Comments
 (0)