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

BroadcastChannel client deleting Y.Map element causes delayed deletion of element in another map. #99

Closed
lucasvanbramer opened this issue Apr 10, 2022 · 9 comments
Assignees
Labels
bug Something isn't working

Comments

@lucasvanbramer
Copy link

Describe the bug
With two clients connected over a broadcastChannel to the same Doc, client1 deleting an element of map1 causes a value in map2 to be deleted on client1's side, but only after the client2 modifies the value of map2. Client1 and client2 then split and their documents do not share state. Client2 will behave normally on its side and client1 has inconsistent behavior depending on what happened in its version of map2.

To Reproduce
This issue is pretty hard to reproduce without having a GUI to emit events manually from each client at the right time. Here is the simplest possible code to illustrate the phenomenon, with comments describing what happens step by step coming from which client:

const example = () =>
{
  // time t=-1:
  // SETUP: indicates structure. 
  // Does not matter who does this setup.
  // as whoever joins the broadcastChannel second 
  // gets the initial state from whoever is first.
  // I do not think this is important, but the level
  // of nesting may be what is causing this?
  const yDoc = new Y.Doc();
  const setupMapDepth0 = yDoc.getMap("setupMapDepth0");
  const setupMapDepth1 = new Y.Map();
  setupMapDepth0.set("setupMapDepth1", setupMapDepth1);
  const setupMapDepth2 = new Y.Map();
  setupMapDepth1.set("setupMap2", setupMapDepth2);

  const map1 = new Y.Map();
  setupMapDepth2.set("map1", map1);
  const map2 = new Y.Map();
  setupMapDepth2.set("map2", map2);

  const itemOne = new Y.Map();
  itemOne.set("text", "text1");
  map1.set("itemOne", itemOne);
  map2.set("counter", 0);

  // time t=0
  // CLIENT ONE does this
  // This is the critical line. 
  // Things only go wrong after a delete happens.
  map1.delete("itemOne");

  // time t=1: 
  // CLIENT TWO does this
  map2.set("counter", 1);

  // time t=2:
  // CLIENT ONE receives a change to counter: {action: 'delete', oldValue: 0}
  // CLIENT TWO receives a change to counter: {action: 'update', oldValue: 0}

  // time t=3:
  // After this point, changes are still successfully sent across the broadcastChannel. 
  // I was able to step through with the debugger but was not able to see any major
  // differences between behavior of readUpdatesV2 on client1 and client2.
  // However, the two clients' states are completely disconnected and remote changes
  // no longer propagate into the document, only local changes do.
};

Expected behavior
At time t=2, we expect both clients one and two to receive the change to count {action: 'update', oldValue: 0}.

Screenshots
None, but would be more than happy to share this happening live.

Environment Information

Additional context
This is happening deep within my app, but again would be happy to share any further details because this has been kind of a blocker (I can work around not having delete functionality, but it would be hacky).

@lucasvanbramer lucasvanbramer added the bug Something isn't working label Apr 10, 2022
@dmonad
Copy link
Member

dmonad commented Apr 10, 2022

Hi @lucasvanbramer,

Can you please log map._item.id in both browser windows and tell me if they are the same? Here, map is the Yjs type that doesn't sync anymore.

@lucasvanbramer
Copy link
Author

lucasvanbramer commented Apr 10, 2022

They are the same and clock values are the same, even after t=2. (After t=2, the client who received the 'delete' on map2 is sometimes still receiving events for map1).

@dmonad
Copy link
Member

dmonad commented Apr 10, 2022

Can you please check the value of map._item.deleted? - for both browser windows

@lucasvanbramer
Copy link
Author

lucasvanbramer commented Apr 10, 2022

For both map1 and map2 (I am observing on both), they both indicate that map1._item.deleted and map2._item.deleted are false.

Perhaps an interesting detail - after the delete of counter on client1, client1's observer of map2 (the counter map) no longer emits events to .observe - I can only see that map2 is not deleted because the observer on map1 is still firing.

@dmonad
Copy link
Member

dmonad commented Apr 14, 2022

You'll probably notice that doc.store.pendingStructs is not null, correct?

The issue that you described can happen when you manipulate the document in Yjs' events (e.g. change a value in the observer-call). This will cause the provider to send an update via broadcastchannel. However, the event will not be sent because I'm still using lib0/mutex to prevent users from propagating updates that they already received.

If you checked that pendingStructs is not null, then I'll try to find a solution for this (low priority). On your end, you can fix the issue by not manipulating the document within observer-calls. While I can fix some issues that result from doing this, it is generally discouraged to manipulate the document within the observer-call because doing it also screws up the order of events in some cases.

@lucasvanbramer
Copy link
Author

You're right - I did notice pendingStructs was not null, and your explanation makes perfect sense. This is a fantastic, clear explanation and was exactly what I was looking for. Thank you so much for this assistance and all the work you've put into Yjs!

@dmonad dmonad closed this as completed in d6df8ad Apr 17, 2022
@dmonad
Copy link
Member

dmonad commented Apr 17, 2022

@lucasvanbramer I fixed the issue in [email protected]

@lucasvanbramer
Copy link
Author

Thank you!

@hesselbom
Copy link

@dmonad This fix unfortunately caused a regression bug wherein my project that uses yjs, y-codemirror and y-websocket suddenly started logging the following for every change/keystroke in CodeMirror:

[yjs] Changed the client-id because another client seems to be using it.

And it looks like every keystroke creates a new client id which then messes up a bunch of other things (e.g. remote cursors, etc.).

I'll try to create a repro when I have the time but just wanted to mention it here so it isn't lost.

For now I'll continue using [email protected] which still works fine.

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

3 participants