-
Notifications
You must be signed in to change notification settings - Fork 173
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RCORE-2108 Avoid deleting self if the instance of the same object is added to a collection in a mixed. #7647
Conversation
for (auto& obj : objs) | ||
CHECK(obj.get_backlink_count() == 1); | ||
|
||
for (auto& obj : objs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this fails:
Assertion failed: begin <= end [2, 1]
0 realm-tests 0x000000010bd8d159
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't get this to fail.
…ollection created from the same table and colkey
Pull Request Test Coverage Report for Build nicola.cabiddu_1676Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comments. And I am pretty sure that this has nothing to do with the linked issue.
// Update size (also in header) | ||
--m_size; | ||
set_header_size(m_size); | ||
if (ndx < size()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say it is an error in the calling code if ndx is not smaller than size.
@@ -969,6 +969,40 @@ TEST(List_NestedCollection_Unresolved) | |||
CHECK_EQUAL(list.get(0), Mixed(obj.get_link())); | |||
} | |||
|
|||
TEST(Collection_Mixed_avoid_deleting_self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code above is not called when this test is run.
for (auto& obj : objs) | ||
CHECK(obj.get_backlink_count() == 1); | ||
|
||
for (auto& obj : objs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't get this to fail.
What, How & Why?
We should not allow this behaviour, probably the right fix is to throw an exception if we add a link to an object that is basically an instance of the same table, if the table has just one mixed column.
The aim is to prevent this issue: #7657
☑️ ToDos
bindgen/spec.yml
, if public C++ API changed