jupyter-server / pycrdt-websocket

WebSocket Connector for pycrdt
https://jupyter-server.github.io/pycrdt-websocket
MIT License
14 stars 10 forks source link

RuntimeError: Already mutably borrowed #79

Open darabos opened 1 month ago

darabos commented 1 month ago

Description

I have a pycrdt.Doc that I make available to clients through a pycrdt_websocket.WebsocketServer. It's contained in a YRoom.

I want to react to changes in this doc on the server side too. I use observe() to be notified of the changes. Upon a change I want to access the contents of the Doc. But this fails:

  File "python3.12/site-packages/pycrdt/_doc.py", line 198, in __getitem__
    return self._roots[key]
           ^^^^^^^^^^^
  File "python3.12/site-packages/pycrdt/_doc.py", line 246, in _roots
    with self.transaction() as txn:
  File "python3.12/site-packages/pycrdt/_transaction.py", line 61, in __enter__
    self._txn = self._doc._doc.create_transaction()
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
RuntimeError: Already mutably borrowed

What am I doing wrong? Does the YRoom have exclusive access to the Doc?

Reproduce

I can try to make an isolated repro if necessary.

Expected behavior

I was hoping I could access the data in the Doc from the observe() event handler.

darabos commented 1 month ago

I get a similar but slightly different error if I try it without WebsocketServer.

import pycrdt

def handle_changes(ydoc):
    print('handle_changes')
    print(ydoc['workspace']['env'])
    print('never reached')

ydoc = pycrdt.Doc()
ydoc['workspace'] = ws = pycrdt.Map()
ydoc['workspace']['env'] = 'unset'
ydoc.observe(lambda event: handle_changes(ydoc))
print('watch out')
ydoc['workspace']['env'] = 'good'

The output:

watch out
handle_changes
Traceback (most recent call last):
  File "python3.12/site-packages/pycrdt/_doc.py", line 113, in _roots
    for key, val in self._doc.roots(txn._txn).items()
                    ^^^^^^^^^^^^^^^^^^^^^^^^^
RuntimeError: Already borrowed

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "repro.py", line 11, in <lambda>
    ydoc.observe(lambda event: handle_changes(ydoc))
                               ^^^^^^^^^^^^^^^^^^^^
  File "repro.py", line 5, in handle_changes
    print(ydoc['workspace']['env'])
          ~~~~^^^^^^^^^^^^^
  File "python3.12/site-packages/pycrdt/_doc.py", line 84, in __getitem__
    return self._roots[key]
           ^^^^^^^^^^^
  File "python3.12/site-packages/pycrdt/_doc.py", line 105, in _roots
    with self.transaction() as txn:
  File "python3.12/site-packages/pycrdt/_transaction.py", line 68, in __exit__
    self._txn.commit()
RuntimeError: Already borrowed

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "repro.py", line 13, in <module>
    ydoc['workspace']['env'] = 'good'
    ~~~~~~~~~~~~~~~~~^^^^^^^
  File "python3.12/site-packages/pycrdt/_map.py", line 88, in __setitem__
    with self.doc.transaction():
SystemError: <function Transaction.__exit__ at 0x1014e0400> returned a result with an exception set
davidbrochart commented 1 month ago

It seems to me that you want to observe changes on the Map rather than on the Doc, right? This would work:

import pycrdt

def handle_changes(event):
    print(event.target['env'])

ydoc = pycrdt.Doc()
ydoc['workspace'] = ws = pycrdt.Map()
ws['env'] = 'unset'
ws.observe(handle_changes)
ws['env'] = 'good'
darabos commented 1 month ago

Wow, thanks! That works great!

When I do the same in my websocket setup, the function registered with ws.observe() never gets called. The function registered with ydoc.observe() is called properly. I hope I can track down why. If not, I'll come back with a new repro!

darabos commented 1 month ago

I construct the empty YDoc with workspace and set observers for both the YDoc and ws.

If I call ydoc.apply_update(b'\x01\x01\xe1\xe7\xa1\xf2\x06\x00(\x01\tworkspace\x03env\x01w\x06Pillow\x00') both observers are called. Perfect!

But if the same update comes from the frontend and ydoc.apply_update() is called by YRoom (via handle_sync_message()) only the YDoc observer is called.

Maybe something happens to the YDoc before the update arrives? I'll try deleting everything that touches YDoc.

Or it's some threading/async thing, where apply_update() is called from somewhere else? I tried allow_multithreading=True but it had no effect. I'm using pycrdt_websocket.ASGIServer() with FastAPI like this:

@router.websocket("/ws/crdt/{room_name}")
async def crdt_websocket(websocket: fastapi.WebSocket, room_name: str):
    await asgi_server({'path': room_name}, websocket._receive, websocket._send)

Anyway, I'll keep digging. I must be close!

darabos commented 1 month ago

I've got something. I think it's destructors deleting subscriptions.

import pycrdt

def handle_changes(event):
    print(event.target['env'])

ydoc = pycrdt.Doc()
ydoc['workspace'] = ws = pycrdt.Map()
ws['env'] = 'unset'
ws.observe(handle_changes)
del ws
ydoc['workspace']['env'] = 'good'

It's the same as your example except for the last two lines. If ws goes out of scope, the observer won't be called. If I delete del ws it's fine. My larger code works now too! I just do room.ws = ws to keep it alive.

I find this behavior surprising, because I didn't think of ws as an object that needs to be kept alive. It just points to a part of ydoc, doesn't it? I could just write ydoc['workspace'].observe(handle_changes) instead. Which also doesn't work, because the Map immediately goes out of scope.

Anyway, thanks a million for PyCRDT! My application will be so collaborative!

davidbrochart commented 1 month ago

The thing is that ydoc['workspace'] is not ws. Under the hood ydoc['workspace'] is a new pycrdt.Map object, which doesn't have an observe callback.