YousefED / SyncedStore

SyncedStore CRDT is an easy-to-use library for building live, collaborative applications that sync automatically.
https://syncedstore.org
MIT License
1.71k stars 51 forks source link

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

Closed pierscowburn closed 1 year ago

pierscowburn commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Great find, this has been fixed now!

pierscowburn commented 1 year ago

Great, thank you!