serramatutu commented on code in PR #3905:
URL: https://github.com/apache/arrow-adbc/pull/3905#discussion_r3720101571
##########
rust/driver_manager/src/lib.rs:
##########
@@ -939,6 +955,31 @@ impl ManagedStatement {
}
}
+struct StatementCancelHandle {
+ inner: std::sync::Weak<ManagedStatementInner>,
+}
+
+impl adbc_core::CancelHandle for StatementCancelHandle {
+ fn try_cancel(&self) -> Result<()> {
+ if let Some(inner) = self.inner.upgrade() {
+ if let AdbcVersion::V100 =
inner.connection.database.driver.version {
+ return Err(Error::with_message_and_status(
+ ERR_CANCEL_UNSUPPORTED,
+ Status::NotImplemented,
+ ));
+ }
+ let driver = &inner.connection.database.driver.driver;
+ let mut statement = inner.statement.lock().unwrap();
Review Comment:
Same as above :)
##########
rust/driver_manager/src/lib.rs:
##########
@@ -565,6 +565,31 @@ pub struct ManagedConnection {
inner: Arc<ManagedConnectionInner>,
}
+struct ConnectionCancelHandle {
+ inner: std::sync::Weak<ManagedConnectionInner>,
+}
+
+impl adbc_core::CancelHandle for ConnectionCancelHandle {
+ fn try_cancel(&self) -> Result<()> {
+ if let Some(inner) = self.inner.upgrade() {
+ if let AdbcVersion::V100 = inner.database.driver.version {
+ return Err(Error::with_message_and_status(
+ ERR_CANCEL_UNSUPPORTED,
+ Status::NotImplemented,
+ ));
+ }
+ let driver = &inner.database.driver.driver;
+ let mut connection = inner.connection.lock().unwrap();
Review Comment:
This adds an expectation that the thread who owns the connection cannot have
panicked (mutex poison) otherwise trying to cancel any statements on that
connection will also panic the canceler thread, right?
Instead of `unwrap()`, could we instead make this return an error so that a
panic in the owning thread doesn't spread to canceler threads and they can deal
with it gracefully?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
