Skip to content
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

XMLElement children manipulation randomly breaks #38

Closed
eliias opened this issue Oct 14, 2022 · 1 comment
Closed

XMLElement children manipulation randomly breaks #38

eliias opened this issue Oct 14, 2022 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@eliias
Copy link
Collaborator

eliias commented Oct 14, 2022

Problem

Manipulating an XMLElement randomly breaks (observed in flaky tests). It is possible that the manipulation isn't done properly on the Ruby side or that we have some memory mapping issues between Ruby and Rust.

Examples

Failures:

  1) Y::XMLElement when changing commits automatically
     Failure/Error: expect(changes[0].first[:added].map(&:tag)).to match_array(%w[A B C])

       expected collection contained:  ["A", "B", "C"]
       actual collection contained:    ["\u0000", "A", "C"]
       the missing elements were:      ["B"]
       the extra elements were:        ["\u0000"]
     # ./spec/y/yxml_element_spec.rb:324:in `block (3 levels) in <top (required)>'

Job: https://github.com/y-crdt/yrb/actions/runs/3234322746/jobs/5297285225

Failures:

  1) Y::XMLElement when changing commits automatically
     Failure/Error: expect(changes[0].first[:added].map(&:tag)).to match_array(%w[A B C])

       expected collection contained:  ["A", "B", "C"]
       actual collection contained:    ["\n", "A", "C"]
       the missing elements were:      ["B"]
       the extra elements were:        ["\n"]
     # ./spec/y/xml_element_spec.rb:324:in `block (3 levels) in <top (required)>'

Job: https://github.com/y-crdt/yrb/actions/runs/3287990605/jobs/5417793161

thread '<unnamed>' panicked at 'called `Option::unwrap()` on a `None` value', /Users/hannesmoser/.cargo/registry/src/github.com-1ecc6299db9ec823/yrs-0.12.0/src/types/xml.rs:110:29
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

fatal: called `Option::unwrap()` on a `None` value

  0) Y::XMLElement when changing commits automatically
     Failure/Error: yxml_element_tag

     fatal:
       called `Option::unwrap()` on a `None` value
     # ./lib/y/xml.rb:237:in `yxml_element_tag'
     # ./lib/y/xml.rb:237:in `tag'
     # ./spec/y/xml_element_spec.rb:320:in `map'
     # ./spec/y/xml_element_spec.rb:320:in `block (3 levels) in <top (required)>'

Analysis

For whatever reason, some of the (XML) elements that are forwarded to Ruby are no longer in memory when we access them. The container is there Y::XMLElement, but when accessing the tag, we get a different set of errors.

pub fn tag(&self) -> &str {
  let inner = &self.0 .0;
  inner.name.as_ref().unwrap()
}

When I add GC.start after the observer is attached, I can reliably produce the issues. I think this confirms that the elements emitted by change events are no longer accessible due to being garbage collected.

# garbage collect immediately
GC.start

There is some documentation about this in magnus. We are required to keep values on the stack, and I think it is impossible, because if we delete an element, it actually gets removed from the CRDT, and we just pass it to the callback and eventually access it later on. In other words, in order for the observer pattern to work, I need to control garbage collection 🤷.
https://github.com/matsadler/magnus#safety

@eliias eliias self-assigned this Oct 14, 2022
@eliias eliias added the bug Something isn't working label Oct 14, 2022
@Iwark Iwark mentioned this issue Oct 21, 2022
@eliias
Copy link
Collaborator Author

eliias commented Jan 19, 2023

This issue seems to not happen anymore at all after the major changes done in the yrs 0.13.x+ implementation.

@eliias eliias closed this as completed Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant