[Mobile][Android][Bug] SIGSEGV if ortSession.close() is called before ortSession.run() finish

[shadowofsoul]

Describe the issue

It took me a while to figure out why the testing in the store keep crashing, until i found this.
If ortSession.close() is called before ortSession.run() finish it causes a SIGSEGV on the next ortSession.run().
This happen even if you nullify the ortSession and create a new one with a diferent model.
Passing RunOption.setTerminate(true) to run() prevents the crashing, but doesn't allow to run any other future ineference.

ortSession.close should somehow, forcefully close the run(), or have a new method to stop the run(), in case the user wants to abort the operation, if not, devs are forced to make the user wait until the whole ineference finish to abort the operation.
Of course, i understand i'm destroying an or closing an object when another thread and operation is using it, but that why one of this functions are needed.

To reproduce

1. Create a ortSession, make a run() ineference on a separated thread.
2. call ortSession.close() before run() finishes.
3. Create a new ortSession and try to run() again on a thread.

Urgency

Is pretty urgent as it blocks the user experience having to wait until the ineference finish, to cancel the operation.

Platform

Android

OS Version

14

ONNX Runtime Installation

Built from Source

Compiler Version (if 'Built from Source')

34

Package Name (if 'Released Package')

onnxruntime-android

ONNX Runtime Version or Commit ID

1.20.0

ONNX Runtime API

Java/Kotlin

Architecture

ARM64

Execution Provider

Default CPU

Execution Provider Library Version

No response

[shadowofsoul]

After a night of sleep, i realized that my request is technically hard and not really in concordance with the structure of onnxruntime. The only way to do this is to make run() be executed in a new thread internally that can be forcefully closed, but i suppose that onnxruntime structure wants to leave the threading management to the end developer and not be managed internally (i agree with this). Worst case scenario the dev can close it's own thread forcefully (not recommended of course, by any good development practice) or make some GUI adjustements.

Still, it would be nice to have a clarification in the documentation, that ortSession.close() doesn't prevent this, is not really clear, as i assumed that close() would stop run() if it was executing it.

my 2 cents.

[Craigacp]

If you need to abort an inference then you should supply a RunOptions and terminate it yourself. Once that terminate call has gone through you should be able to close the session and create a fresh one. If you blindly close it we'd need to track all the in flight run calls in the session object, add our own RunOptions and terminate them ourselves, which is a lot of additional overhead that most developers don't want.

We can make the documentation modification, and I am interested in the case where calling terminate prevents you from creating a new session, because that sounds like a bug somewhere.

[shadowofsoul]

If you need to abort an inference then you should supply a RunOptions and terminate it yourself. Once that terminate call has gone through you should be able to close the session and create a fresh one. If you blindly close it we'd need to track all the in flight run calls in the session object, add our own RunOptions and terminate them ourselves, which is a lot of additional overhead that most developers don't want.

We can make the documentation modification, and I am interested in the case where calling terminate prevents you from creating a new session, because that sounds like a bug somewhere.

Thanks for the answer. Answering, i passed RunOptions with setTermine(true) on the new created ortSession.run() just ends with an exception that the inerence ended (but doesn't crash).

I'll create and submit a test project to reproduce.

[Craigacp]

The exception is the only way that call can end so at the moment you need to catch it and special case the handling logic to check if it's from the terminate flag (e.g. https://github.com/microsoft/onnxruntime/blob/main/java/src/test/java/ai/onnxruntime/InferenceTest.java#L1214), but I agree it would be nicer to have it be a subclass of OrtException that can be easily caught separately from the other more "error-y" exceptions that can come out of a session.run call.