Skip to content

Commit

Permalink
Merge pull request #467 from JetBrains/ab-fix-uninitialized-property-…
Browse files Browse the repository at this point in the history
…transfer

[rd-cpp] Fix serialization of uninitialized properties.
  • Loading branch information
mirasrael authored Jan 16, 2024
2 parents 2b05bd0 + a754182 commit 60d88cb
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 13 deletions.
21 changes: 15 additions & 6 deletions rd-cpp/src/rd_framework_cpp/src/main/impl/RdProperty.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,20 +43,29 @@ class RdProperty final : public RdPropertyBase<T, S>, public ISerializable
static RdProperty<T, S> read(SerializationCtx& ctx, Buffer& buffer)
{
RdId id = RdId::read(buffer);
bool not_null = buffer.read_bool(); // not null/
(void) not_null;
auto value = S::read(ctx, buffer);
RdProperty<T, S> property;
property.value = std::move(value);
withId(property, id);
const bool has_value = buffer.read_bool();
if (has_value)
{
auto value = S::read(ctx, buffer);
property.value = std::move(value);
}
return property;
}

void write(SerializationCtx& ctx, Buffer& buffer) const override
{
this->rdid.write(buffer);
buffer.write_bool(true);
S::write(ctx, buffer, this->get());
if (this->has_value())
{
buffer.write_bool(true);
S::write(ctx, buffer, this->get());
}
else
{
buffer.write_bool(false);
}
}

void advise(Lifetime lifetime, std::function<void(T const&)> handler) const override
Expand Down
5 changes: 5 additions & 0 deletions rd-cpp/src/rd_framework_cpp/src/main/task/RdTaskResult.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,11 @@ class RdTaskResult final : public ISerializable
return v.index() == 0;
}

Success& as_successful()
{
return rd::get<Success>(v);
}

bool is_canceled() const
{
return v.index() == 1;
Expand Down
14 changes: 7 additions & 7 deletions rd-cpp/src/rd_framework_cpp/src/main/task/WiredRdTaskImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,15 @@ class WiredRdTaskImpl : public RdReactiveBase
LifetimeImpl::counter_t termination_lifetime_id{};

template <class Bindable = T, std::enable_if_t<util::is_bindable_v<Bindable>, bool> = true>
TaskResult bind_result(TaskResult task_result) const
void bind_result(TaskResult& task_result) const
{
if (!task_result.is_succeeded())
return task_result;
return;

auto lifetime_defintion = LifetimeDefinition(lifetime);
auto result_lifetime = lifetime_defintion.lifetime;
auto value = util::attach_lifetime(task_result.get_value(), std::move(lifetime_defintion));
auto& success = task_result.as_successful();
auto value = util::attach_lifetime(success.value, std::move(lifetime_defintion));
result_lifetime->add_action([task_id = get_id(), cutpoint = cutpoint]
{
cutpoint->get_wire()->send(task_id, [](auto&)
Expand All @@ -43,13 +44,12 @@ class WiredRdTaskImpl : public RdReactiveBase
});
});
value->bind(result_lifetime, cutpoint, "CallResult");
return typename TaskResult::Success(value);
success.value = value;
}

template <class NonBindable = T, std::enable_if_t<!util::is_bindable_v<NonBindable>, bool> = true>
TaskResult bind_result(TaskResult result) const
void bind_result(TaskResult&) const
{
return result;
}

public:
Expand Down Expand Up @@ -85,7 +85,7 @@ class WiredRdTaskImpl : public RdReactiveBase
}
else
{
result = bind_result(std::move(result));
bind_result(result);
this->result->set_if_empty(std::move(result));
}
});
Expand Down
30 changes: 30 additions & 0 deletions rd-cpp/src/rd_framework_cpp/src/test/cases/RdTaskTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,5 +204,35 @@ TEST_F(RdFrameworkTestBase, testAsyncBindableCall)
EXPECT_TRUE(server_result_lifetime_terminated) << "Expected server lifetime for result to be terminated.";
EXPECT_TRUE(server_result.unique()) << "Expected server_result to be released. Test should hold only reference to server result.";

AfterTest();
}

TEST_F(RdFrameworkTestBase, testUninitializedPropertyInResult)
{
RdEndpoint<std::wstring, test::util::DynamicEntity> server_entity;
RdCall<std::wstring, test::util::DynamicEntity> client_entity;

statics(server_entity, static_entity_id);
statics(client_entity, static_entity_id);

Wrapper<test::util::DynamicEntity> server_result;

server_entity.set([&](std::wstring const&)
{
server_result = wrapper::make_wrapper<test::util::DynamicEntity>();
return server_result;
});

bindStatic(serverProtocol.get(), server_entity, static_name);
bindStatic(clientProtocol.get(), client_entity, static_name);

auto task_result = client_entity.start(L"xy").value_or_throw();
auto client_result = task_result.get_value();
auto& foo_property = client_result->get_foo();
EXPECT_THROW(foo_property.get(), std::exception) << "Expected to throw when access unitialized property";

server_result->get_foo().set(2);
EXPECT_EQ(2, foo_property.get()) << "Expected to sync property value when it set on server.";

AfterTest();
}

0 comments on commit 60d88cb

Please sign in to comment.