Skip to content

Commit 4a0b95d

Browse files
njzjznjzjz-botpre-commit-ci[bot]njzjz-bot
authored
fix(tf): close C++ API sessions (#5800)
Closes #5657. ## Summary - initialize the TensorFlow `Session*` member to `nullptr` in `DeepPotTF`, `DeepSpinTF`, `DeepTensorTF`, and `DipoleChargeModifierTF`; - centralize exclusive session teardown as `Close()` followed by `delete` and pointer reset; - use an initialization rollback guard so public `init()` calls are failure-atomic and can be retried without overwriting a session left by an earlier failed attempt; - clean sessions from both initializing-constructor exception paths and normal destructors; - make `DeepPotJAX` initialization failure-atomic by unconditionally releasing all partially created TensorFlow C API handles (`TF_Session`, `TF_Graph`, `TF_SessionOptions`, `TF_Status`, `TFE_Context`, `TFE_ContextOptions`, and `TF_Function` references); - add direct lifecycle regressions for failed initialization/retry and successful initialization/normal destruction. ## Root cause The TensorFlow wrappers owned raw sessions created by `NewSession`, but their destructors and initializing-constructor catch blocks only deleted the `GraphDef`. Default construction followed by a failing public `init()` could also retain a partially initialized session, allowing a retry to overwrite the pointer. `DeepPotJAX` had the same failure mode with TensorFlow C API handles: its destructor only released resources when `inited == true`, so an exception before the final initialization commit leaked every handle created up to that point. The TensorFlow rollback guard now owns cleanup until initialization has completely committed. `DeepPotJAX::clear_tf_resources()` is null-safe for partial initialization and is called both from the exception path and the destructor. Successful initialization commits only after all metadata has been loaded. ## Test coverage The lifecycle test intentionally white-box-links `deepmd_backend_tf` and `deepmd_backend_jax` into `runUnitTests_cc`, rather than going through the `dlopen` plugin facade. This makes the concrete backend destructors directly visible to the LSan test process. The tests cover: - repeated failed initialization followed by safe destruction for all four TensorFlow C++ wrappers; - successful initialization followed by normal destruction for all four TensorFlow C++ wrappers; - repeated failed `DeepPotJAX` initialization; - successful `DeepPotJAX` initialization followed by normal destruction when the generated SavedModel test artifact is available. Ordinary builds verify retry safety and destructor behavior; the CI leak-sanitizer leg provides the direct leak regression signal. ## Validation - built `deepmd_backend_tf`, `deepmd_backend_jax`, `runUnitTests_cc`, and `deepmd_op` in a TensorFlow/JAX C++ test configuration with LAMMPS plugin mode disabled; - `TestTensorFlowSessionLifecycle.*`: 3 passed, 1 skipped locally because the generated JAX SavedModel artifact was unavailable; - `ruff check .`; - `ruff format .`; - clang-format check; - CMake formatting hook; - `git diff --check`. Coding agent: Codex Codex version: codex-cli 0.144.4 Model: gpt-5.6-sol Reasoning effort: xhigh --------- Co-authored-by: njzjz-bot <njzjz-bot@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: njzjz-bot <njzjz.bot@gmail.com>
1 parent 91b722c commit 4a0b95d

9 files changed

Lines changed: 409 additions & 152 deletions

File tree

source/api_cc/include/DeepPotJAX.h

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,13 @@ class DeepPotJAX : public DeepPotBackend {
242242
const bool atomic);
243243

244244
private:
245+
/** Release every TensorFlow C API handle owned by this backend.
246+
*
247+
* This helper is intentionally safe for partially initialized objects so an
248+
* exception from init() cannot leak handles or make a later retry unsafe.
249+
*/
250+
void clear_tf_resources() noexcept;
251+
245252
bool inited;
246253
// device
247254
std::string device;
@@ -284,12 +291,12 @@ class DeepPotJAX : public DeepPotBackend {
284291
/** TF C API objects.
285292
* @{
286293
*/
287-
TF_Graph* graph;
288-
TF_Status* status;
289-
TF_Session* session;
290-
TF_SessionOptions* sessionopts;
291-
TFE_ContextOptions* ctx_opts;
292-
TFE_Context* ctx;
294+
TF_Graph* graph = nullptr;
295+
TF_Status* status = nullptr;
296+
TF_Session* session = nullptr;
297+
TF_SessionOptions* sessionopts = nullptr;
298+
TFE_ContextOptions* ctx_opts = nullptr;
299+
TFE_Context* ctx = nullptr;
293300
std::vector<TF_Function*> func_vector;
294301
/**
295302
* @}

source/api_cc/include/tf_private.h

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,49 @@ typedef tensorflow::tstring STRINGTYPE;
1818
#else
1919
typedef std::string STRINGTYPE;
2020
#endif
21+
22+
/**
23+
* @brief Close and release an exclusively owned TensorFlow session.
24+
*
25+
* TensorFlow permits concurrent Run calls, but requires them all to finish
26+
* before the sole Close call. Owners must therefore stop concurrent use
27+
* before destruction; this helper only centralizes the non-throwing cleanup
28+
* needed by destructors and partially constructed API objects.
29+
*
30+
* @param[in,out] session Exclusively owned session, reset to nullptr.
31+
*/
32+
inline void close_and_delete_session(tensorflow::Session*& session) noexcept {
33+
if (session == nullptr) {
34+
return;
35+
}
36+
session->Close().IgnoreError();
37+
delete session;
38+
session = nullptr;
39+
}
40+
41+
/**
42+
* @brief Roll back a session assigned during a potentially throwing init call.
43+
*
44+
* Call release() only after initialization has completed successfully.
45+
*/
46+
class SessionCleanupGuard {
47+
public:
48+
explicit SessionCleanupGuard(tensorflow::Session*& session) noexcept
49+
: session_(session), active_(true) {}
50+
51+
~SessionCleanupGuard() {
52+
if (active_) {
53+
close_and_delete_session(session_);
54+
}
55+
}
56+
57+
SessionCleanupGuard(const SessionCleanupGuard&) = delete;
58+
SessionCleanupGuard& operator=(const SessionCleanupGuard&) = delete;
59+
60+
void release() noexcept { active_ = false; }
61+
62+
private:
63+
tensorflow::Session*& session_;
64+
bool active_;
65+
};
2166
} // namespace deepmd

source/api_cc/src/DataModifierTF.cc

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,29 @@ using namespace deepmd;
88
using namespace tensorflow;
99

1010
DipoleChargeModifierTF::DipoleChargeModifierTF()
11-
: inited(false), graph_def(new GraphDef()) {}
11+
: session(nullptr), graph_def(new GraphDef()), inited(false) {}
1212

1313
DipoleChargeModifierTF::DipoleChargeModifierTF(const std::string& model,
1414
const int& gpu_rank,
1515
const std::string& name_scope_)
16-
: inited(false), name_scope(name_scope_), graph_def(new GraphDef()) {
16+
: session(nullptr),
17+
name_scope(name_scope_),
18+
graph_def(new GraphDef()),
19+
inited(false) {
1720
try {
1821
init(model, gpu_rank, name_scope_);
1922
} catch (...) {
2023
// Clean up and rethrow, as the destructor will not be called
24+
deepmd::close_and_delete_session(session);
2125
delete graph_def;
2226
throw;
2327
}
2428
}
2529

26-
DipoleChargeModifierTF::~DipoleChargeModifierTF() { delete graph_def; };
30+
DipoleChargeModifierTF::~DipoleChargeModifierTF() {
31+
deepmd::close_and_delete_session(session);
32+
delete graph_def;
33+
};
2734

2835
void DipoleChargeModifierTF::init(const std::string& model,
2936
const int& gpu_rank,
@@ -34,6 +41,7 @@ void DipoleChargeModifierTF::init(const std::string& model,
3441
<< std::endl;
3542
return;
3643
}
44+
deepmd::SessionCleanupGuard session_guard(session);
3745
name_scope = name_scope_;
3846
SessionOptions options;
3947
get_env_nthreads(num_intra_nthreads, num_inter_nthreads);
@@ -72,6 +80,7 @@ void DipoleChargeModifierTF::init(const std::string& model,
7280
get_vector<int>(sel_type, "model_attr/sel_type");
7381
sort(sel_type.begin(), sel_type.end());
7482
inited = true;
83+
session_guard.release();
7584
}
7685

7786
template <class VT>

0 commit comments

Comments
 (0)