Skip to content

Commit b7bfdf7

Browse files
apolyakovapolyakov
committed
[k2] fix error with double store of tl::mask in tl::rpcReqResultExtra
1 parent b61435f commit b7bfdf7

6 files changed

Lines changed: 48 additions & 75 deletions

File tree

‎common/tl/constants/common.h‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -45,16 +45,16 @@ namespace vk {
4545
namespace tl {
4646
namespace common {
4747

48-
namespace tracing::traceContext {
49-
static constexpr uint32_t return_reserved_status_0 = 1 << 0;
50-
static constexpr uint32_t return_reserved_status_1 = 1 << 1;
51-
static constexpr uint32_t parent_id = 1 << 2;
52-
static constexpr uint32_t source_id = 1 << 3;
53-
static constexpr uint32_t return_reserved_level_0 = 1 << 4;
54-
static constexpr uint32_t return_reserved_level_1 = 1 << 5;
55-
static constexpr uint32_t return_reserved_level_2 = 1 << 6;
56-
static constexpr uint32_t return_debug = 1 << 7;
57-
} // namespace tracing::traceContext
48+
namespace tracing::trace_context_flags {
49+
inline constexpr uint32_t reserved_status_0 = 1U << 0U;
50+
inline constexpr uint32_t reserved_status_1 = 1U << 1U;
51+
inline constexpr uint32_t parent_id = 1U << 2U;
52+
inline constexpr uint32_t source_id = 1U << 3U;
53+
inline constexpr uint32_t reserved_level_0 = 1U << 4U;
54+
inline constexpr uint32_t reserved_level_1 = 1U << 5U;
55+
inline constexpr uint32_t reserved_level_2 = 1U << 6U;
56+
inline constexpr uint32_t debug = 1U << 7U;
57+
} // namespace tracing::trace_context_flags
5858

5959
namespace rpc_invoke_req_extra_flags {
6060
inline constexpr uint32_t return_binlog_pos = 1U << 0U;

‎common/tl/tl-types.h‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -126,14 +126,14 @@ struct traceID final {
126126
};
127127

128128
class traceContext final {
129-
static constexpr uint32_t RETURN_RESERVED_STATUS_0_FLAG = vk::tl::common::tracing::traceContext::return_reserved_status_0;
130-
static constexpr uint32_t RETURN_RESERVED_STATUS_1_FLAG = vk::tl::common::tracing::traceContext::return_reserved_status_1;
131-
static constexpr uint32_t PARENT_ID_FLAG = vk::tl::common::tracing::traceContext::parent_id;
132-
static constexpr uint32_t SOURCE_ID_FLAG = vk::tl::common::tracing::traceContext::source_id;
133-
static constexpr uint32_t RETURN_RESERVED_LEVEL_0_FLAG = vk::tl::common::tracing::traceContext::return_reserved_level_0;
134-
static constexpr uint32_t RETURN_RESERVED_LEVEL_1_FLAG = vk::tl::common::tracing::traceContext::return_reserved_level_1;
135-
static constexpr uint32_t RETURN_RESERVED_LEVEL_2_FLAG = vk::tl::common::tracing::traceContext::return_reserved_level_2;
136-
static constexpr uint32_t RETURN_DEBUG_FLAG = vk::tl::common::tracing::traceContext::return_debug;
129+
static constexpr uint32_t RESERVED_STATUS_0_FLAG = vk::tl::common::tracing::trace_context_flags::reserved_status_0;
130+
static constexpr uint32_t RESERVED_STATUS_1_FLAG = vk::tl::common::tracing::trace_context_flags::reserved_status_1;
131+
static constexpr uint32_t PARENT_ID_FLAG = vk::tl::common::tracing::trace_context_flags::parent_id;
132+
static constexpr uint32_t SOURCE_ID_FLAG = vk::tl::common::tracing::trace_context_flags::source_id;
133+
static constexpr uint32_t RESERVED_LEVEL_0_FLAG = vk::tl::common::tracing::trace_context_flags::reserved_level_0;
134+
static constexpr uint32_t RESERVED_LEVEL_1_FLAG = vk::tl::common::tracing::trace_context_flags::reserved_level_1;
135+
static constexpr uint32_t RESERVED_LEVEL_2_FLAG = vk::tl::common::tracing::trace_context_flags::reserved_level_2;
136+
static constexpr uint32_t DEBUG_FLAG = vk::tl::common::tracing::trace_context_flags::debug;
137137

138138
public:
139139
tracing::traceID trace_id{};
@@ -169,12 +169,12 @@ class traceContext final {
169169
ok = ok && !tl_fetch_error();
170170
}
171171

172-
reserved_status_0 = static_cast<bool>(fields_mask & RETURN_RESERVED_STATUS_0_FLAG);
173-
reserved_status_1 = static_cast<bool>(fields_mask & RETURN_RESERVED_STATUS_1_FLAG);
174-
reserved_level_0 = static_cast<bool>(fields_mask & RETURN_RESERVED_LEVEL_0_FLAG);
175-
reserved_level_1 = static_cast<bool>(fields_mask & RETURN_RESERVED_LEVEL_1_FLAG);
176-
reserved_level_2 = static_cast<bool>(fields_mask & RETURN_RESERVED_LEVEL_2_FLAG);
177-
debug_flag = static_cast<bool>(fields_mask & RETURN_DEBUG_FLAG);
172+
reserved_status_0 = static_cast<bool>(fields_mask & RESERVED_STATUS_0_FLAG);
173+
reserved_status_1 = static_cast<bool>(fields_mask & RESERVED_STATUS_1_FLAG);
174+
reserved_level_0 = static_cast<bool>(fields_mask & RESERVED_LEVEL_0_FLAG);
175+
reserved_level_1 = static_cast<bool>(fields_mask & RESERVED_LEVEL_1_FLAG);
176+
reserved_level_2 = static_cast<bool>(fields_mask & RESERVED_LEVEL_2_FLAG);
177+
debug_flag = static_cast<bool>(fields_mask & DEBUG_FLAG);
178178

179179
return ok;
180180
}

‎runtime-light/server/rpc/init-functions.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ void init_server(kphp::component::stream&& request_stream, kphp::stl::vector<std
210210
superglobals.v$_SERVER.set_value(string{RPC_ACTOR_ID.data(), RPC_ACTOR_ID.size()}, (*invoke_rpc.opt_actor_id).value);
211211
}
212212
if (invoke_rpc.opt_extra) {
213-
superglobals.v$_SERVER.set_value(string{RPC_EXTRA_FLAGS.data(), RPC_EXTRA_FLAGS.size()}, static_cast<int64_t>((*invoke_rpc.opt_extra).get_flags().value));
213+
superglobals.v$_SERVER.set_value(string{RPC_EXTRA_FLAGS.data(), RPC_EXTRA_FLAGS.size()}, static_cast<int64_t>((*invoke_rpc.opt_extra).flags.value));
214214
process_rpc_invoke_req_extra(*invoke_rpc.opt_extra, superglobals);
215215
}
216216
kphp::log::info("rpc server initialized with: "
@@ -222,7 +222,7 @@ void init_server(kphp::component::stream&& request_stream, kphp::stl::vector<std
222222
"request -> {:#x}",
223223
invoke_rpc.net_pid.get_pid(), invoke_rpc.net_pid.get_port(), invoke_rpc.query_id.value,
224224
invoke_rpc.opt_actor_id.has_value() ? (*invoke_rpc.opt_actor_id).value : 0,
225-
invoke_rpc.opt_extra.has_value() ? (*invoke_rpc.opt_extra).get_flags().value : 0, request_magic.value);
225+
invoke_rpc.opt_extra.has_value() ? (*invoke_rpc.opt_extra).flags.value : 0, request_magic.value);
226226
}
227227

228228
} // namespace kphp::rpc

‎runtime-light/stdlib/rpc/rpc-api.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ inline kphp::coro::task<> f$rpc_server_store_response(class_instance<C$VK$TL$Rpc
269269
// as we are in a coroutine, we must own the data to prevent it from being overwritten by another coroutine,
270270
// so create a TLBuffer owned by this coroutine
271271
auto& rpc_server_instance_st{RpcServerInstanceState::get()};
272-
tl::K2RpcResponse rpc_response{.value = tl::k2RpcResponseHeader{.flags = {}, .extra = {}, .result = rpc_server_instance_st.tl_storer.view()}};
272+
tl::K2RpcResponse rpc_response{.value = tl::k2RpcResponseHeader{.extra = {}, .result = rpc_server_instance_st.tl_storer.view()}};
273273
tl::storer tls{rpc_response.footprint()};
274274
rpc_response.store(tls);
275275

‎runtime-light/tl/tl-types.cpp‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,6 @@ bool CertInfoItem::fetch(tl::fetcher& tlf) noexcept {
147147
// ===== RPC =====
148148

149149
bool rpcInvokeReqExtra::fetch(tl::fetcher& tlf) noexcept {
150-
tl::mask flags{};
151150
bool ok{flags.fetch(tlf)};
152151
if (ok && static_cast<bool>(flags.value & WAIT_BINLOG_POS_FLAG)) {
153152
ok &= opt_wait_binlog_pos.emplace().fetch(tlf);
@@ -194,31 +193,6 @@ bool rpcInvokeReqExtra::fetch(tl::fetcher& tlf) noexcept {
194193
return ok;
195194
}
196195

197-
tl::mask rpcInvokeReqExtra::get_flags() const noexcept {
198-
tl::mask flags{.value = static_cast<tl::mask::underlying_type>(return_binlog_pos)};
199-
200-
flags.value |= static_cast<tl::mask::underlying_type>(return_binlog_time) << 1;
201-
flags.value |= static_cast<tl::mask::underlying_type>(return_pid) << 2;
202-
flags.value |= static_cast<tl::mask::underlying_type>(return_request_sizes) << 3;
203-
flags.value |= static_cast<tl::mask::underlying_type>(return_failed_subqueries) << 4;
204-
flags.value |= static_cast<tl::mask::underlying_type>(return_query_stats) << 6;
205-
flags.value |= static_cast<tl::mask::underlying_type>(no_result) << 7;
206-
flags.value |= static_cast<tl::mask::underlying_type>(return_view_number) << 27;
207-
208-
flags.value |= static_cast<tl::mask::underlying_type>(opt_wait_binlog_pos.has_value()) << 16;
209-
flags.value |= static_cast<tl::mask::underlying_type>(opt_string_forward_keys.has_value()) << 18;
210-
flags.value |= static_cast<tl::mask::underlying_type>(opt_int_forward_keys.has_value()) << 19;
211-
flags.value |= static_cast<tl::mask::underlying_type>(opt_string_forward.has_value()) << 20;
212-
flags.value |= static_cast<tl::mask::underlying_type>(opt_int_forward.has_value()) << 21;
213-
flags.value |= static_cast<tl::mask::underlying_type>(opt_custom_timeout_ms.has_value()) << 23;
214-
flags.value |= static_cast<tl::mask::underlying_type>(opt_supported_compression_version.has_value()) << 25;
215-
flags.value |= static_cast<tl::mask::underlying_type>(opt_random_delay.has_value()) << 26;
216-
flags.value |= static_cast<tl::mask::underlying_type>(opt_persistent_query.has_value()) << 28;
217-
flags.value |= static_cast<tl::mask::underlying_type>(opt_trace_context.has_value()) << 29;
218-
flags.value |= static_cast<tl::mask::underlying_type>(opt_execution_context.has_value()) << 30;
219-
return flags;
220-
}
221-
222196
void rpcReqResultExtra::store(tl::storer& tls) const noexcept {
223197
flags.store(tls);
224198
if (static_cast<bool>(flags.value & BINLOG_POS_FLAG)) {

‎runtime-light/tl/tl-types.h‎

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,14 +1008,14 @@ struct traceID final {
10081008
};
10091009

10101010
class traceContext final {
1011-
static constexpr uint32_t RETURN_RESERVED_STATUS_0_FLAG = vk::tl::common::tracing::traceContext::return_reserved_status_0;
1012-
static constexpr uint32_t RETURN_RESERVED_STATUS_1_FLAG = vk::tl::common::tracing::traceContext::return_reserved_status_1;
1013-
static constexpr uint32_t PARENT_ID_FLAG = vk::tl::common::tracing::traceContext::parent_id;
1014-
static constexpr uint32_t SOURCE_ID_FLAG = vk::tl::common::tracing::traceContext::source_id;
1015-
static constexpr uint32_t RETURN_RESERVED_LEVEL_0_FLAG = vk::tl::common::tracing::traceContext::return_reserved_level_0;
1016-
static constexpr uint32_t RETURN_RESERVED_LEVEL_1_FLAG = vk::tl::common::tracing::traceContext::return_reserved_level_1;
1017-
static constexpr uint32_t RETURN_RESERVED_LEVEL_2_FLAG = vk::tl::common::tracing::traceContext::return_reserved_level_2;
1018-
static constexpr uint32_t RETURN_DEBUG_FLAG = vk::tl::common::tracing::traceContext::return_debug;
1011+
static constexpr uint32_t RESERVED_STATUS_0_FLAG = vk::tl::common::tracing::trace_context_flags::reserved_status_0;
1012+
static constexpr uint32_t RESERVED_STATUS_1_FLAG = vk::tl::common::tracing::trace_context_flags::reserved_status_1;
1013+
static constexpr uint32_t PARENT_ID_FLAG = vk::tl::common::tracing::trace_context_flags::parent_id;
1014+
static constexpr uint32_t SOURCE_ID_FLAG = vk::tl::common::tracing::trace_context_flags::source_id;
1015+
static constexpr uint32_t RESERVED_LEVEL_0_FLAG = vk::tl::common::tracing::trace_context_flags::reserved_level_0;
1016+
static constexpr uint32_t RESERVED_LEVEL_1_FLAG = vk::tl::common::tracing::trace_context_flags::reserved_level_1;
1017+
static constexpr uint32_t RESERVED_LEVEL_2_FLAG = vk::tl::common::tracing::trace_context_flags::reserved_level_2;
1018+
static constexpr uint32_t DEBUG_FLAG = vk::tl::common::tracing::trace_context_flags::debug;
10191019

10201020
public:
10211021
tl::tracing::traceID trace_id{};
@@ -1036,23 +1036,23 @@ class traceContext final {
10361036
bool debug_flag{};
10371037

10381038
bool fetch(tl::fetcher& tlf) noexcept {
1039-
tl::u32 fields_mask{};
1039+
tl::mask fields_mask{};
10401040
bool ok{fields_mask.fetch(tlf)};
10411041

10421042
ok = ok && trace_id.fetch(tlf);
10431043
if (ok && static_cast<bool>(fields_mask.value & PARENT_ID_FLAG)) {
1044-
ok = ok && opt_parent_id.emplace().fetch(tlf);
1044+
ok &= opt_parent_id.emplace().fetch(tlf);
10451045
}
10461046
if (ok && static_cast<bool>(fields_mask.value & SOURCE_ID_FLAG)) {
1047-
ok = ok && opt_source_id.emplace().fetch(tlf);
1047+
ok &= opt_source_id.emplace().fetch(tlf);
10481048
}
10491049

1050-
reserved_status_0 = static_cast<bool>(fields_mask.value & RETURN_RESERVED_STATUS_0_FLAG);
1051-
reserved_status_1 = static_cast<bool>(fields_mask.value & RETURN_RESERVED_STATUS_1_FLAG);
1052-
reserved_level_0 = static_cast<bool>(fields_mask.value & RETURN_RESERVED_LEVEL_0_FLAG);
1053-
reserved_level_1 = static_cast<bool>(fields_mask.value & RETURN_RESERVED_LEVEL_1_FLAG);
1054-
reserved_level_2 = static_cast<bool>(fields_mask.value & RETURN_RESERVED_LEVEL_2_FLAG);
1055-
debug_flag = static_cast<bool>(fields_mask.value & RETURN_DEBUG_FLAG);
1050+
reserved_status_0 = static_cast<bool>(fields_mask.value & RESERVED_STATUS_0_FLAG);
1051+
reserved_status_1 = static_cast<bool>(fields_mask.value & RESERVED_STATUS_1_FLAG);
1052+
reserved_level_0 = static_cast<bool>(fields_mask.value & RESERVED_LEVEL_0_FLAG);
1053+
reserved_level_1 = static_cast<bool>(fields_mask.value & RESERVED_LEVEL_1_FLAG);
1054+
reserved_level_2 = static_cast<bool>(fields_mask.value & RESERVED_LEVEL_2_FLAG);
1055+
debug_flag = static_cast<bool>(fields_mask.value & DEBUG_FLAG);
10561056

10571057
return ok;
10581058
}
@@ -1070,6 +1070,7 @@ class traceContext final {
10701070
return flags;
10711071
}
10721072
};
1073+
10731074
} // namespace tracing
10741075

10751076
class rpcInvokeReqExtra final {
@@ -1094,6 +1095,7 @@ class rpcInvokeReqExtra final {
10941095
static constexpr uint32_t EXECUTION_CONTEXT_FLAG = vk::tl::common::rpc_invoke_req_extra_flags::execution_context;
10951096

10961097
public:
1098+
tl::mask flags{};
10971099
bool return_binlog_pos{};
10981100
bool return_binlog_time{};
10991101
bool return_pid{};
@@ -1115,8 +1117,6 @@ class rpcInvokeReqExtra final {
11151117
bool return_view_number{};
11161118

11171119
bool fetch(tl::fetcher& tlf) noexcept;
1118-
1119-
tl::mask get_flags() const noexcept;
11201120
};
11211121

11221122
struct RpcInvokeReqExtra final {
@@ -1180,16 +1180,15 @@ struct k2RpcResponseError final {
11801180
};
11811181

11821182
struct k2RpcResponseHeader final {
1183-
tl::mask flags{};
11841183
tl::rpcReqResultExtra extra{};
11851184
std::span<const std::byte> result;
11861185

11871186
void store(tl::storer& tls) const noexcept {
1188-
flags.store(tls), extra.store(tls), tls.store_bytes(result);
1187+
extra.store(tls), tls.store_bytes(result);
11891188
}
11901189

11911190
constexpr size_t footprint() const noexcept {
1192-
return flags.footprint() + extra.footprint() + result.size();
1191+
return extra.footprint() + result.size();
11931192
}
11941193
};
11951194

0 commit comments

Comments
 (0)