rust: `cancel` is effectively useless since everything is `&mut self`

[lidavidm]

What feature or improvement would you like to see?

You can't call it at all because of this.

[lidavidm]

CC @eitsupi @felipecrv @mbrobbel for thoughts. Is there a more Rust-native API we should be using (e.g. cancellation tokens)? (Though for FFI, then we need some way to be notified when the cancellation token is triggered so that we can make the appropriate FFI call; also I'm guessing there is no ecosystem-wide cancellation token like the way Go has context.Context)

[felipecrv]

I was talking about this issue with @xuliangs yesterday! For now, my plan is to do an unsound transmute.

My take is that Rust code should call cancel() with a non-mut & and have drivers take care of the threading issues internally. cancel() is a way of passing a message to the thread that started the execute().

[lidavidm]

I think we'd also have to mark everything else as &self instead of &mut self, right?

I was thinking maybe we should instead have a get_cancel_token(&self) that you call in advance, which splits the object into something with a cancel and something without. (And have drivers deal with how to handle this internally.) I'm not sure what would feel natural to implement for native-Rust drivers though. (Possibly if we go with this, it would just wrap a cancellation token?)

[felipecrv]

This is a nice way of convincing rustc nothing wrong is being done and drivers written in Rust could store some internal ID instead of aliasing the mutable pointer to the Statement like it's possible in Go/C/C++. A con is that it adds yet another object to the ADBC protocol.

Another idea:

let conn = database.new_connection();
...
let stmt = conn.new_statement();
... 
let stmt_id = stmt.id();  // standardize on a 128-bit byte string
...
let reader = stmt.execute();

While another thread can:

database.cancel_stmt(stmt_id);

Connections remain thread-confined. The database can track the statements for cancelling them.

[felipecrv]

I think we should also consider that what is safe for other languages should also be safe for Rust. Unsound code is not acceptable in Go and C++ just because the compiler doesn't complain as much. We just have to enforce the guarantees manually.

The calls that the Rust FFI makes should not be against &mut references if driver objects are expected to implement safe internal mutability. Just like I can acquire a lock from a const & in Rust and make &mut calls on the object, we should also be allowed to make FFI calls through & and expect the driver to manage the thread-safety on their side.

We are just adding yet another layer of locking in Rust code on top of the one that should exist in drivers such that they work correctly in non-Rust applications.

The alternative to that is clearly documenting the constraints that every driver manager implementation should put on top of the drivers they use, but that reduces the freedom of implementation for drivers. Depending on what drivers do to talk to the data sources, different multi-threaded programming patterns can be used.

Similar discussion has happened before in #2791

[lidavidm]

Well, just marking &self isn't enough without also marking as Sync which may impose too much on implementations (most/all drivers are only Send even in other languages with the exception of cancellation). (Note that Go uses a token for cancellation so this isn't even relevant there.) I think the separate handle more clearly expresses what was intended, too.

[mbrobbel]

+1 for internal mutability

[lidavidm]

@mbrobbel to be clear that is "mark everything as &self and Sync"?

[mbrobbel]

@mbrobbel to be clear that is "mark everything as &self and Sync"?

Just &self. I don't see why would need to require Sync?

[lidavidm]

You can't call it otherwise, right? Because the statement would be locked to the thread that called execute and you wouldn't be able to share it.

[lidavidm]

(This is also why I kind of prefer the separate handle, as it matches the C/C++ API more closely: most methods can't be called concurrently, except cancel which is the intentional exception for obvious reasons.)

[mbrobbel]

You can't call it otherwise, right? Because the statement would be locked to the thread that called execute and you wouldn't be able to share it.

I guess it depends on the implementation?

If we remove all &mut methods on Connection and Statement, implementations can use a handler-like type for statements.