rocicorp / repc

The canonical Replicache client, implemented in Rust.
Other
30 stars 7 forks source link

Argh #361

Open arv opened 3 years ago

sayrer commented 3 years ago

I looked at this for a while over coffee this morning. I got a lot further by doing this:

let txn :Transaction = open_txn(name, store, mutator_args, rebase_opts).await?; where open_txn is an async function that contains the big match statement.

(edit:: I just had a pun error on kv::Store vs store::Store before. Do a straightforward conversion to that line above, and you'll get to

error[E0515]: cannot return value referencing local variable `store`
   --> src/wasm.rs:173:13
    |
167 |             let dag_read = store.read(lc.clone()).await.map_err(DagReadError)?;
    |                            ----- `store` is borrowed here
...
173 |             Ok(Transaction::Read(read))
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ returns a value referencing data owned by the current function

but that gets you past those lifetime errors, where lexically scoped things fall out of async scope. A version of this issue: https://rust-lang.github.io/async-book/03_async_await/01_chapter.html#async-lifetimes.

I kept looking at this until I got to a problem with insert_transaction(txn_id: u32, txn: Transaction<'static>) and the lifetimes on the transaction data.

sayrer commented 3 years ago

I ended up getting these errors:

error[E0759]: `dag_read` has lifetime `'a` but it needs to satisfy a `'static` lifetime requirement
   --> src/wasm.rs:133:26
    |
133 | async fn do_read_txn<'a>(dag_read: dag::read::OwnedRead<'a>) -> Result<u32, OpenTransactionError> {
    |                          ^^^^^^^^  ------------------------ this data with lifetime `'a`...
    |                          |
    |                          ...is captured here...
...
143 |     insert_transaction(txn_id, Transaction::Read(read));
    |                                ----------------------- ...and is required to live as long as `'static` here

error[E0759]: `dag_write` has lifetime `'a` but it needs to satisfy a `'static` lifetime requirement
   --> src/wasm.rs:149:5
    |
149 |     dag_write: Write<'a>,
    |     ^^^^^^^^^  --------- this data with lifetime `'a`...
    |     |
    |     ...is captured here...
...
166 |     insert_transaction(txn_id, Transaction::Write(write));
    |                                ------------------------- ...and is required to live as long as `'static` here

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0759`.
error: could not compile `replicache-client`

after writing:


async fn do_read_txn<'a>(dag_read: dag::read::OwnedRead<'a>) -> Result<u32, OpenTransactionError> {
    use OpenTransactionError::*;

    let read = db::OwnedRead::from_whence(
        db::Whence::Head(db::DEFAULT_HEAD_NAME.to_string()),
        dag_read,
    )
    .await
    .map_err(DBReadError)?;
    let txn_id = TRANSACTION_COUNTER.fetch_add(1, Ordering::SeqCst);
    insert_transaction(txn_id, Transaction::Read(read));
    // txns.write().await.insert(txn_id, RwLock::new(txn));
    Ok(txn_id)
}

async fn do_write_txn<'a>(
    dag_write: Write<'a>,
    mutator_name: String,
    mutator_args: String,
    rebase_opts: Option<RebaseOpts>,
) -> Result<u32, OpenTransactionError> {
    let (whence, original_hash) = match rebase_opts {
        None => (db::Whence::Head(db::DEFAULT_HEAD_NAME.to_string()), None),
        Some(opts) => {
            validate_rebase(&opts, dag_write.read(), &mutator_name, &mutator_args).await?;
            (db::Whence::Hash(opts.basis), Some(opts.original_hash))
        }
    };

    let write = db::Write::new_local(whence, mutator_name, mutator_args, original_hash, dag_write)
        .await
        .map_err(OpenTransactionError::DBWriteError)?;
    let txn_id = TRANSACTION_COUNTER.fetch_add(1, Ordering::SeqCst);
    insert_transaction(txn_id, Transaction::Write(write));
    // txns.write().await.insert(txn_id, RwLock::new(txn));
    Ok(txn_id)
}

#[wasm_bindgen]
pub async fn open_transaction(
    db_name: String,
    name: Option<String>,
    mutator_args: Option<String>,
    basis: Option<String>,
    original_hash: Option<String>,
) -> Result<u32, js_sys::Error> {
    use OpenTransactionError::*;

    let rebase_opts = match (basis, original_hash) {
        (None, None) => None,
        (None, Some(_)) | (Some(_), None) => panic!("Invalid arguments"),
        (Some(basis), Some(original_hash)) => Some(RebaseOpts {
            basis,
            original_hash,
        }),
    };

    let lc = LogContext::new();

    let tx = match name {
        Some(name) => {
            let mutator_args = mutator_args.ok_or(OpenTransactionError::ArgsRequired)?;
            let lc = LogContext::new();

            let lock_timer = rlog::Timer::new();
            debug!(lc, "Waiting for write lock...");
            let store =
                get_dag_store(&db_name).ok_or(js_sys::Error::new("Database is not open"))?;
            let dag_write = store.write(lc.clone()).await.map_err(DagWriteError)?;
            debug!(
                lc,
                "...Write lock acquired in {}ms",
                lock_timer.elapsed_ms()
            );
            do_write_txn(dag_write, name, mutator_args, rebase_opts).await
        }
        None => {
            let store =
                get_dag_store(&db_name).ok_or(js_sys::Error::new("Database is not open"))?;
            let dag_read = store.read(lc.clone()).await.map_err(DagReadError)?;
            do_read_txn(dag_read).await
        }
    };

    tx.map_err(|_e| js_sys::Error::new("Database is not open"))
}
arv commented 3 years ago

@sayrer Thanks for looking at this. Maybe I should back up and outline the goal here.

The goal is to drive more of the "dispatch loop" from JS. JS would do all the calling and "scheduling". Rust/Wasm cannot be pure functions because we need to keep some state around between these calls (transactions for example). I was planning to keep that state as a thread local (from an architecture point it would also work if it was global since JS is single threaded) in a map and then look up the Rust objects based on an ID of some kind that is returned to JS and passed in when JS calls into Rust/Wasm.

sayrer commented 3 years ago

I see. I'm not sure I understand the problem well enough (I don't know the data structures very well). That said, I think you might want Transaction data without a lifetime parameter, or perhaps you could alter this function to return a js_sys::Promise using future_to_promise

(No worries on the context, I sometimes look at repc on Sunday mornings to see if there are any weird bugs, I don't mind if I fail)