amoeba commented on code in PR #4858: URL: https://github.com/apache/arrow-adbc/pull/4858#discussion_r4221652136
########## rust/driver_manager/src/lib.rs: ########## @@ -93,29 +93,33 @@ // According to the ADBC specification, objects allow serialized access from // multiple threads: one thread may make a call, and once finished, another // thread may make a call. They do not allow concurrent access from multiple -// threads. +// threads, except that cancellation must be able to overlap ordinary calls. // -// In order to implement these semantics, all mutable FFI objects are wrapped -// in `Mutex`es. `FFI_Driver` is not wrapped in a `Mutex` because it is +// Databases use a `Mutex`. Connections and statements use `NativeHandle` to +// serialize ordinary calls while allowing cancellation through raw pointers. +// Their shared inner objects release the native handles only after all calls +// (including cancellation) have relinquished their strong references. +// `FFI_Driver` is not wrapped in a `Mutex` because it is // an immutable struct of function pointers. Wrapping the driver in a `Mutex` // would prevent any parallelism between driver calls, which is not desirable. Review Comment: nit: re-hard-wrap this. Not sure if this will work right (I did it from the github UI) ```suggestion // Databases use a `Mutex`. Connections and statements use `NativeHandle` to // serialize ordinary calls while allowing cancellation through raw pointers. // Their shared inner objects release the native handles only after all calls // (including cancellation) have relinquished their strong references. // `FFI_Driver` is not wrapped in a `Mutex` because it is an immutable struct of // function pointers. Wrapping the driver in a `Mutex` would prevent any // parallelism between driver calls, which is not desirable. ``` -- 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]
