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

RuntimeError: Already mutably borrowed #79

Open
darabos opened this issue Oct 14, 2024 · 6 comments
Open

RuntimeError: Already mutably borrowed #79

darabos opened this issue Oct 14, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@darabos
Copy link
Contributor

darabos commented Oct 14, 2024

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 darabos added the bug Something isn't working label Oct 14, 2024
@darabos
Copy link
Contributor Author

darabos commented Oct 14, 2024

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
Copy link
Collaborator

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
Copy link
Contributor Author

darabos commented Oct 14, 2024

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
Copy link
Contributor Author

darabos commented Oct 14, 2024

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
Copy link
Contributor Author

darabos commented Oct 14, 2024

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
Copy link
Collaborator

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.

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

2 participants