mw/com[Bug]: fix infinite recursion in ProxyEvent move-assignment#399
mw/com[Bug]: fix infinite recursion in ProxyEvent move-assignment#399KrishaDeshkool wants to merge 1 commit intoeclipse-score:mainfrom
Conversation
KrishaDeshkool
requested review from
LittleHuba,
bemerybmw,
castler,
crimson11,
hoe-jo and
limdor
as code owners
KrishaDeshkool
changed the title
| if (this != &other) | ||
| { | ||
| ProxyEvent::operator=(std::move(other)); | ||
| ProxyEventBase::operator=(std::move(other)); |
There was a problem hiding this comment.
ProxyEventBase::operator(std::move(static_cast<ProxyEventBase&&>(other)));
|
|
||
| using SampleType = std::uint16_t; | ||
|
|
||
| ProxyBase empty_proxy{std::make_unique<mock_binding::Proxy>(), |
There was a problem hiding this comment.
Why not use the existing fixture?
|
|
||
| auto first_binding_ptr = std::make_unique<StrictMock<mock_binding::ProxyEvent<SampleType>>>(); | ||
| auto second_binding_ptr = std::make_unique<StrictMock<mock_binding::ProxyEvent<SampleType>>>(); | ||
| auto& first_binding = *first_binding_ptr; |
There was a problem hiding this comment.
We generally use ProxyEventFacade instead of this approach.
|
|
||
| // Then subsequent calls on the destination dispatch to the source's binding (proving the binding was transferred) | ||
| constexpr std::size_t max_sample_count{7U}; | ||
| EXPECT_CALL(first_binding, GetSubscriptionState()).WillOnce(Return(SubscriptionState::kNotSubscribed)); |
There was a problem hiding this comment.
expectations should go above the when section
There was a problem hiding this comment.
Does this test pass without the change in proxy_event?
There was a problem hiding this comment.
no, it crashes with back trace posted in another comment
|
|
||
| TEST(ProxyEventTest, MoveAssignmentTransfersBindingFromSourceToDestination) | ||
| { | ||
| RecordProperty("Verifies", "SCR-5897869"); // SWS_CM_00135 |
There was a problem hiding this comment.
What requirement are you linking here?
There was a problem hiding this comment.
it is a copy paste from the one above which checked this "IsMovable"
There was a problem hiding this comment.
What is actually happening in the old code? I can't imagine that there was actually infinite recursion because we would have seen it when moving a proxy.
There was a problem hiding this comment.
you can run this without changing the move op
gdb --batch --ex 'run --gtest_filter=ProxyEventTest.MoveAssignmentTransfersBindingFromSourceToDestination' --ex 'bt 10' bazel-bin/score/mw/com/impl/proxy_event_test
then you will see this
Program received signal SIGSEGV, Segmentation fault.
0x0000555555604d32 in score::mw::com::impl::ProxyEvent::operator=(score::mw::com::impl::ProxyEvent&&) & ()
#0 0x0000555555604d32 in score::mw::com::impl::ProxyEvent::operator=(score::mw::com::impl::ProxyEvent&&) & ()
#1 0x0000555555604d5f in score::mw::com::impl::ProxyEvent::operator=(score::mw::com::impl::ProxyEvent&&) & ()
#2 0x0000555555604d5f in score::mw::com::impl::ProxyEvent::operator=(score::mw::com::impl::ProxyEvent&&) & ()
#3 0x0000555555604d5f in score::mw::com::impl::ProxyEvent::operator=(score::mw::com::impl::ProxyEvent&&) & ()
#4 0x0000555555604d5f in score::mw::com::impl::ProxyEvent::operator=(score::mw::com::impl::ProxyEvent&&) & ()
#5 0x0000555555604d5f in score::mw::com::impl::ProxyEvent::operator=(score::mw::com::impl::ProxyEvent&&) & ()
#6 0x0000555555604d5f in score::mw::com::impl::ProxyEvent::operator=(score::mw::com::impl::ProxyEvent&&) & ()
#7 0x0000555555604d5f in score::mw::com::impl::ProxyEvent::operator=(score::mw::com::impl::ProxyEvent&&) & ()
#8 0x0000555555604d5f in score::mw::com::impl::ProxyEvent::operator=(score::mw::com::impl::ProxyEvent&&) & ()
#9 0x0000555555604d5f in score::mw::com::impl::ProxyEvent::operator=(score::mw::com::impl::ProxyEvent&&) & ()
q551424@clid2082740:~/score/communication (fix_move_bug)$ gdb --batch --ex 'run --gtest_filter=ProxyEventTest.MoveAssignmentTransfersBindingFromSourceToDestination' --ex 'bt 10' bazel-bin/score/mw/com/impl/proxy_event_test
KrishaDeshkool
force-pushed
the
fix_move_bug
branch
from
8b73a99 to
b18b7a1
Compare
2 participants
No description provided.