serizba / cppflow

Run TensorFlow models in C++ without installation and without Bazel
https://serizba.github.io/cppflow/
MIT License
788 stars 178 forks source link

Fix error when destructing a static std::unique_ptr<cppflow::model> #132

Closed ihowell closed 2 years ago

ihowell commented 3 years ago

Fixes #131

When creating a static version of a reference to a cppflow::model, there becomes an error when destructing, due to the context::get_status() method using a thread_local status. While this is done normally for performance gains and is fine in most places. However, when the program is exiting, it first destructs thread_local objects and then static ones. This leads to local_tf_status containing a nullptr instead of a pointer to a TF_Status object. Therefore, TF_DeleteSession(sess, status) fails because it access *status. The simplest fix is to not re-use this status object in the destructor of the session and instead make a new, non-thread_local status instead.

serizba commented 2 years ago

Hi @ihowel, thanks for spotting this!

Perhaps is better to allocate on TF_Status per model, as suggested by @ljn917 here.

ljn917 commented 2 years ago

@serizba Do you mind me having a PR to fix, i.e. adding a per model TF_Status? The global status will be used for eager ops only. There were so many people trying to build cppflow into a dll and having the deinitialuzation order fiasco.

serizba commented 2 years ago

@ljn917 Yeah sure! This way we'll fix all related issues.

Thanks!