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

Double borrow in observe_after_transaction #69

Closed
Waidhoferj opened this issue Jul 9, 2022 · 2 comments · Fixed by #73
Closed

Double borrow in observe_after_transaction #69

Waidhoferj opened this issue Jul 9, 2022 · 2 comments · Fixed by #73
Assignees
Labels
bug Something isn't working

Comments

@Waidhoferj
Copy link
Collaborator

Key YDoc methods are inaccessible inside observe_after_transaction callbacks due to a double borrow. For example

def test_borrow_issue():
    d = Y.YDoc()
    m = d.get_map("foo")
    update: bytes = None

    def put_updates(ydoc: Y.YDoc, event) -> None:
        nonlocal update
        update = Y.encode_state_as_update(ydoc, event.before_state) #borrowed again here FAILS

    d.observe_after_transaction(lambda e: put_updates(d, e)) # Borrowed here

    with d.begin_transaction() as txn:
        m.set(txn, "hi", "there")
@Waidhoferj
Copy link
Collaborator Author

Waidhoferj commented Jul 11, 2022

So here's the issue: AfterTransactionEvents are fired before the transaction is freed, so that the developer has access to transaction-related attributes in the callback. The transaction holds a mutable reference to its associated YDoc so it can update the data inside. When you call encode_state_as_update, the function uses the provided YDoc to create a second new transaction. In doing so, it borrows YDoc again mutably, which isn't permitted because there cannot be 2 mutable references to the same element according to Rust ownership rules.

In #73, I added a way to do the same thing from the AfterTransactionEvent passed to the callback. Its generally a better idea to use the event if possible. @davidbrochart lmk if you need any additional functionality from the AfterTransactionEvent.

@davidbrochart
Copy link
Collaborator

For now this is all I need, thanks!

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

Successfully merging a pull request may close this issue.

2 participants