Session thread safety

[marshallpierce]

Thanks for the handy library, first of all.

My read of https://github.com/microsoft/onnxruntime/blob/master/docs/InferenceHighLevelDesign.md#key-design-decisions is that a session is thread safe. Should onnxruntime::session::Session therefore be Send and Sync?

[nbigaouette]

I remember asking myself a similar question last summer. I can't recall if it was about the session though, maybe it was 🤔 . The issue was that I could not see anything that was proving me that the session was threadsafe. Being paranoid and rust-spoiled, I did not opt-in to mark it Send+Sync.

I guess the session can be wrapped in a Arc<Mutex<Session>> to make it Send+Sync?

[krlohnes]

I was about to open a similar issue, and came across this issue.

I guess the session can be wrapped in a Arc<Mutex> to make it Send+Sync?

Fwiw, Arc<Mutex<Session>> won't be Send + Sync. Mutex requires the type to be Sync to also make it Send.

At the moment, the following fails to compile

fn main() {
  let environment = Environment::builder()
      .build()?;
  let mut session = environment
      .new_session_builder()?
      .with_model_from_file("squeezenet1.0-8.onnx")?;
  foo(&session);
}

fn foo<T: Sync> (s: &T) {
...
}

with an error like

error[E0277]: `*mut onnxruntime_sys::OrtSession` cannot be shared between threads safely
   --> src/main.rs:7:6
    |
7  |         foo(&session);
    |         ^^^ `*mut onnxruntime_sys::OrtSession` cannot be shared between threads safely
...
10 | fn foo<T: Sync> (s: &T) {
    |           ---- required by this bound in `foo`
    |
    = help: within `onnxruntime::session::Session`, the trait `std::marker::Sync` is not implemented for `*mut onnxruntime_sys::OrtSession`
    = note: required because it appears within the type `onnxruntime::session::Session`

So you'd need to explicitly add unsafe Sync and unsafe Send impls to the Session struct as far as I can tell.

[marshallpierce]

Multiple threads can invoke the Run() method on the same inference session object. See API doc for more details.

That seems pretty clear to me 🤷, but maybe the other operations on Session should not be called from multiple threads, in which case perhaps some other type (like Runner in #40) might be the right thing to make Send + Sync?

[HaoboGu]

I'm trying to use lazy_static to wrap environment and session, and I met the problem too:

lazy_static! {
    static ref ENVIRONMENT: Environment = Environment::builder().with_name("env").build().unwrap();

    static ref SESSION: Session<'static> = JAVA_ENVIRONMENT.new_session_builder().unwrap().
		.with_optimization_level(GraphOptimizationLevel::Basic).unwrap().
		.with_number_threads(2).unwrap().
		.with_model_from_file("/Users/haobogu/Projects/rust/mymodel.onnx").unwrap();
}

I got a complier error:

error[E0277]: `*mut onnxruntime_sys::OrtSession` cannot be shared between threads safely
  --> src/server/types.rs:10:1
   |
10 | / lazy_static! {
11 | |     static ref ENVIRONMENT: Environment = Environment::builder().with_name("env").build().unwrap();
12 | |
13 | |     static ref SESSION: Session<'static> = JAVA_ENVIRONMENT.new_session_builder().unwrap().
...  |
16 | |         .with_model_from_file("/Users/haobogu/Projects/rust/mymodel.onnx").unwrap();
17 | | }
   | |_^ `*mut onnxruntime_sys::OrtSession` cannot be shared between threads safely
   | 
  ::: /Users/haobogu/.cargo/registry/src/github.com-1ecc6299db9ec823/lazy_static-1.4.0/src/inline_lazy.rs:19:20
   |
19 |   pub struct Lazy<T: Sync>(Cell<Option<T>>, Once);
   |                      ---- required by this bound in `lazy_static::lazy::Lazy`
   |
   = help: within `onnxruntime::session::Session<'static>`, the trait `Sync` is not implemented for `*mut onnxruntime_sys::OrtSession`
   = note: required because it appears within the type `onnxruntime::session::Session<'static>`
   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

It seems a similar situation. @marshallpierce @krlohnes do you have any ideas?

[marshallpierce]

Looks like the same core issue. I wish I had a week or two to dedicate to this library. Ah well...

[failable]

Any news?

[JinghuiZhou]

Any news?