Yrs crashes on Intel machines

[fbarbat]

We first hit this crash in our app. In our app, we found that just the simple fact of adding or removing class to our project, would cause the crash to happen or not. 🤯 The crash would happen consistently or not for a given revision of the code base.

After doing some research, we saw that running the YSwift unit tests for Documents from an Intel MacBook Pro 15-inch, 2019, with macOS 14, also crashes. It's the same stack trace.

Thread 1 Queue : com.apple.main-thread (serial)
#0	0x00000001035df6bf in rand_chacha::guts::refill_wide::impl_avx2::h761b24f35cb5e3f2 ()
#1	0x0000000103624642 in _$LT$yrs..doc..Options$u20$as$u20$core..default..Default$GT$::default::h8fa0eaf813c43622 ()
#2	0x00000001035c71c2 in uniffi_uniffi_yniffi_fn_constructor_yrsdoc_new ()
#3	0x000000010356f77e in closure #1 in YrsDoc.init() at /yswift/lib/swift/scaffold/yniffi.swift:668
#4	0x000000010356a25e in makeRustCall<τ_0_0>(_:errorHandler:) at /yswift/lib/swift/scaffold/yniffi.swift:257
#5	0x000000010356865b in rustCall<τ_0_0>(_:) at /yswift/lib/swift/scaffold/yniffi.swift:242
#6	0x000000010356f6e0 in YrsDoc.__allocating_init() at /yswift/lib/swift/scaffold/yniffi.swift:667
#7	0x00000001035aa536 in YDocument.init() at /yswift/Sources/YSwift/YDocument.swift:13
#8	0x00000001035aa3f1 in YDocument.__allocating_init() ()
#9	0x000000010354db2f in YDocumentTests.test_memoryLeaks() at /yswift/Tests/YSwiftTests/YDocumentTests.swift:7
#10	0x000000010354dd5c in @objc YDocumentTests.test_memoryLeaks() ()

I tried investigating further but I didn't get far since I am not a Rust developer.

• It seems that rand_chacha, a dependency of rand, is crashing.
• This happened on latest master of YSwift (0a42fee) which uses yrs 0.17.4, which uses rand 0.7.0. There's rand 0.8.5. Maybe it's worth trying to update the dependency to see if that helps?

Thank you!

[heckj]

Thanks for the report!

I don't have an intel machine handy to be able to replicate this myself - but the stack trace definitely shows that it's marched into the Rust core library. Are you by chance able to build a local XCFramework instance off the latest main branch and reproduce this today with main?

The part that's causing the issue is a dependency on the main core of Yrs (the core library) at https://github.com/y-crdt/y-crdt/ - looks like that dependency is aligned at https://github.com/y-crdt/y-crdt/blob/main/yrs/Cargo.toml#L18

What I'm not 100% clear on is how to make a branch of Yrs and then depend on it locally for YSwift to test at the swift level if that would resolve the issue. I might even be worthwhile to see if you can run all the relevant code and tests in Rust on that Intel Mac, at least to see if the error is showing up and reproducible with the tests that are there.

If you have the rust toolchains already installed, the quick stab-to-see-if-it-breaks would be something along the lines of:

git clone https://github.com/y-crdt/y-crdt
cd y-crdt
cargo test

If this is an issue with the Rust library, those tests are rather extensions and I'm hoping that their unit tests will fail on that same machine in a way that can get us closer to resolving this for you.

edit: the tests (running cargo test) took about 4 1/2 minutes to run on my M1 MacBook Pro - so expect that won't be a fast process

[heckj]

/cc @Horusiath in case this rings any bells...

[Horusiath]

The reason for using rand=v0.7 is that it supported WASM. IIRC this support was dropped later on, and I didn't look thoroughly into reasons why. Maybe it's a good time to revisit.

[fbarbat]

Thank you for your suggestions.

I pulled latest main and fully recompiled the framework from the Intel machine and the crash is still there.

Regarding y-crdt cargo test from an Intel machine, it succeeds:

test tests::edit_traces_tests::edit_trace_rustcode ... ok
test tests::edit_traces_tests::edit_trace_automerge has been running for over 60 seconds
test tests::edit_traces_tests::edit_trace_sephblog1 has been running for over 60 seconds
test tests::edit_traces_tests::edit_trace_sephblog1 ... ok
test tests::edit_traces_tests::edit_trace_automerge ... ok

test result: ok. 251 passed; 0 failed; 3 ignored; 0 measured; 0 filtered out; finished in 359.64s

     Running unittests src/lib.rs (target/debug/deps/ywasm-2ddbc391f06f242d)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/web.rs (target/debug/deps/web-e9600ea9a2b267b7)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests yrs

running 29 tests
test yrs/src/atomic.rs - atomic::AtomicRef (line 16) ... ok
test yrs/src/any.rs - any::any (line 513) ... ok
test yrs/src/lib.rs - (line 152) ... ok
test yrs/src/lib.rs - (line 18) ... ok
test yrs/src/doc.rs - doc::Doc (line 33) ... ok
test yrs/src/block.rs - block::BlockRange::slice (line 1153) ... ok
test yrs/src/lib.rs - (line 268) ... ok
test yrs/src/lib.rs - (line 361) ... ok
test yrs/src/lib.rs - (line 106) ... ok
test yrs/src/lib.rs - (line 241) ... ok
test yrs/src/lib.rs - (line 295) ... ok
test yrs/src/moving.rs - moving::StickyIndex (line 380) ... ok
test yrs/src/lib.rs - (line 187) ... ok
test yrs/src/moving.rs - moving::StickyIndex::get_offset (line 457) ... ok
test yrs/src/types/array.rs - types::array::Array::move_range_to (line 272) ... ok
test yrs/src/types/array.rs - types::array::ArrayRef (line 37) ... ok
test yrs/src/types/map.rs - types::map::MapRef (line 25) ... ok
test yrs/src/types/text.rs - types::text::Text::diff (line 360) ... ok
test yrs/src/types/text.rs - types::text::Text::insert (line 166) ... ok
test yrs/src/types/text.rs - types::text::Text::insert (line 184) ... ok
test yrs/src/types/text.rs - types::text::TextRef (line 45) ... ok
test yrs/src/types/weak.rs - types::weak::Quotable::quote (line 657) ... ok
test yrs/src/types/weak.rs - types::weak::WeakRef (line 48) ... ok
test yrs/src/types/weak.rs - types::weak::WeakRef<P>::try_deref_value (line 263) ... ok
test yrs/src/types/weak.rs - types::weak::WeakRef<TextRef>::get_string (line 172) ... ok
test yrs/src/types/weak.rs - types::weak::WeakRef<XmlTextRef>::get_string (line 200) ... ok
test yrs/src/types/xml.rs - types::xml::XmlFragment::successors (line 837) ... ok
test yrs/src/types/xml.rs - types::xml::XmlTextRef (line 339) ... ok
test yrs/src/undo.rs - undo::UndoManager<M>::reset (line 316) ... ok

test result: ok. 29 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 7.70s

   Doc-tests ywasm

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

[fbarbat]

@heckj I could make Yswift to depend on a local build of yrs by using:

yrs = { path = "../../y-crdt/yrs" }

Building against master didn't compile so I just checked out 0.17.4 on yrs which is the version Yswift was using.
The crash still happened.
I will try to update rand now to see how it goes.

[fbarbat]

Updating rand to 0.8.5 fixed the issue! As @Horusiath suggested, I had to remove the feature support to wasm-bindgen to make it compile though, and I am not sure about the implications of that. Why does yrs need that?

[Horusiath]

@fbarbat yrs is used in WebAssembly modules ie. as part of ywasm NPM package. I'll prepare some workaround that will make upgrade possible.

[fbarbat]

Thank you @Horusiath!

I still don't understand, though, why the Yrs tests just worked and the crash only happened in the context of Swift/Rust integration. This might mean there's something else that's not working well. In any case, updating dependencies is usually good so I hope the workaround to keep wasm compatibility is not too bad!

Thank you!

[fbarbat]

Hello, I saw that the conflicting dependency was removed from Yrs in the above commit so I wanted to try again to see if that helped. So I tried updating Yrs to 0.18.2 in Yswift but I got stuck with the subscription type changes (I think from this PR). Any ideas about how to update Yswift to the new format or when Yswift will be updated to use 0.18.2+?
Thank you!

[heckj]

@Horusiath I just tried the push this forward myself, and saw a few errors, mostly that Branch is now private. Is there a replacement that we should be using, or should this be made public in the Yrs api?

error[E0603]: struct `Branch` is private
  --> src/array.rs:6:17
   |
6  | use yrs::types::Branch;
   |                 ^^^^^^ private struct
   |
note: the struct `Branch` is defined here
  --> /Users/heckj/.cargo/registry/src/index.crates.io-6f17d22bba15001f/yrs-0.18.2/src/types/mod.rs:16:21
   |
16 | use crate::branch::{Branch, BranchPtr};
   |                     ^^^^^^

The other error that appeared was:

error[E0107]: struct takes 0 lifetime arguments but 1 lifetime argument was supplied
   --> src/undo.rs:113:36
    |
113 |     inner: &'static mut yrs::undo::Event<'static, u64>,
    |                                    ^^^^^ ------- help: remove this lifetime argument
    |                                    |
    |                                    expected 0 lifetime arguments
    |
note: struct defined here, with 0 lifetime parameters
   --> /Users/heckj/.cargo/registry/src/index.crates.io-6f17d22bba15001f/yrs-0.18.2/src/undo.rs:634:12
    |
634 | pub struct Event<M> {
    |            ^^^^^

I can easily pull off the lifetime arguments, but wanted to verify that this should happen.

[fbarbat]

@heckj The errors about subscriptions I mentioned showed up after I "fixed" (not sure how reliably) the ones you are mentioning. Really cool to see that you already are pushing this forward!
I "fixed" the branch one by using use yrs::branch::Branch; and the other one deleting that argument. I followed (maybe incorrectly) what the compiler suggested. And then I hit the subscription ones.
Thank you for pushing this forward! And let me know if I can help with anything.

[Horusiath]

@heckj, @fbarbat is right - Branch was moved to yrs::branch::Branch, while in undo::Event we got rid of lifetime param (it was used as 'static before, so pretty much useless anyway).

[fbarbat]

I hacked the subscriptions around and I am not hitting the crash! Trying to clean it up now and making sure it works well to see if I can share it with you. But I broke backwards compatibility by removing the unobserve methods so I am not sure it will be mergeable as it is.

[heckj]

@fbarbat Put up a PR, and we can iterate to get it in to resolve your crasher!

[fbarbat]

Ok, let me clean it up first. There's one last issue about the undo manager I need to figure out. I had commented it out to check if the crash was still happening. Specifically, this line doesn't compile. What would be the new Yrs way of doing it?

e.item.meta = delegate.call(YrsUndoEvent::new(e), e.item.meta);

no field item on type &mut yrs::undo::Event<u64>
unknown field

[fbarbat]

This seemed to make the tests pass (again, I am not a rust developer so no quite sure this is right)

*e.meta_mut() = delegate.call(YrsUndoEvent::new(e), *e.meta_mut());

Cleaning the PR overall now...

[fbarbat]

Created PR! Let me know what you think. Thank you!

[heckj]

resolved with #44