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

Overwriting an array re-patches array methods, eventually resulting in stack overflow #76

Closed
pierscowburn opened this issue Oct 7, 2022 · 3 comments Β· Fixed by #84
Closed
Assignees

Comments

@pierscowburn
Copy link

pierscowburn commented Oct 7, 2022

First let me say – thanks for an amazing library! It's been really useful for our project πŸ™Œ

I've tracked down an issue related to array usage within SyncedStore objects, which eventually results in stack overflow. Here's the shortest example I could create that demonstrates the issue:

import { syncedStore } from "@syncedstore/core";

type StoreRoot = {
  foo: Foo;
};

type Foo = {
  bar?: number[];
};

const store = syncedStore<StoreRoot>({ foo: {} });

const count = 100000;

for (let i = 0; i < count; ++i) {
  if (i % 1000 === 0) {
    console.log(`${i} of ${count}`);
  }

  // Results in patchGetter('length') being called on the array, even though it's already been patched
  store.foo.bar = [42];

  // Once the loop has executed enough times, reading the length will result in stack overflow
  store.foo.bar.length;
}

Output:

0 of 100000
1000 of 100000
2000 of 100000
3000 of 100000
4000 of 100000
5000 of 100000
6000 of 100000
@reactivedata/reactive:372
  get: function get(target, key, receiver) {
                   ^

RangeError: Maximum call stack size exceeded
    at Object.get (@reactivedata/reactive:372:20)
    at observable (@reactivedata/reactive:287:13)
    at Atom.reportObserved (@reactivedata/reactive:561:12)
    at reportSelfAtom (@syncedstore/yjs-reactive-bindings:111:14)
    at YArray.descriptor.get (@syncedstore/yjs-reactive-bindings:185:9)
    at YArray.descriptor.get (@syncedstore/yjs-reactive-bindings:188:25)
    at YArray.descriptor.get (@syncedstore/yjs-reactive-bindings:188:25)
    at YArray.descriptor.get (@syncedstore/yjs-reactive-bindings:188:25)
    at YArray.descriptor.get (@syncedstore/yjs-reactive-bindings:188:25)
    at YArray.descriptor.get (@syncedstore/yjs-reactive-bindings:188:25)

In our application we run into this quite regularly during any periods of extended usage. Please let me know if I can be of any further help!

@YousefED
Copy link
Owner

Thanks! This definitely looks like a bug and I'll dive into this.

Glad to here SyncedStore has been useful to your project, curious to hear what you're building!

@YousefED
Copy link
Owner

Great find, this has been fixed now!

@pierscowburn
Copy link
Author

Great, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants