Add graph reinit on failure#4237
Open
atobiszei wants to merge 11 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds automatic MediaPipe graph reinitialization on inference failure when using the graph queue/pool, to prevent returning an errored (“poisoned”) graph instance back to the pool for reuse.
Changes:
- Introduces
GraphHelper::reinitialize()plus an RAIIGraphReinitGuardthat rebuilds a freshCalculatorGraphunless dismissed on the success path. - Hooks the guard into queue-based unary and streaming inference paths to trigger graph rebuild after failures.
- Moves per-graph shutdown logic into
GraphHelper’s destructor and simplifiesGraphQueuedestructor.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/mediapipe_internal/mediapipegraphexecutor.hpp |
Adds GraphReinitGuard around queue-path inference and dismisses it on success. |
src/mediapipe_internal/graphqueue.hpp |
Adds GraphHelper destructor, reinitialize() declaration, and new GraphReinitGuard type. |
src/mediapipe_internal/graphqueue.cpp |
Implements GraphHelper shutdown + reinit logic; defaults GraphQueue destructor. |
Comments suppressed due to low confidence (2)
src/mediapipe_internal/graphqueue.cpp:142
- reinitialize() sets graph to nullptr on ObserveOutputStream failure (graph.reset()). The next request acquiring this GraphHelper will dereference graphHelper->graph in GraphIdGuard (graph(*graphHelper->graph)), which can crash if graph is null. Ensure reinitialize() always leaves a non-null graph, or update acquisition logic to detect nullptr and rebuild/replace before use.
if (!absStatus.ok()) {
SPDLOG_ERROR("Graph reinitialize: ObserveOutputStream failed: {}", absStatus.ToString());
graph.reset();
return;
}
src/mediapipe_internal/graphqueue.cpp:162
- reinitialize() sets graph to nullptr on StartRun failure (graph.reset()). The next request acquiring this GraphHelper will dereference graphHelper->graph in GraphIdGuard (graph(*graphHelper->graph)), which can crash if graph is null. Ensure reinitialize() always leaves a non-null graph, or update acquisition logic to detect nullptr and rebuild/replace before use.
if (!absStatus.ok()) {
SPDLOG_ERROR("Graph reinitialize: StartRun failed: {}", absStatus.ToString());
graph.reset();
return;
}
Comment on lines
+93
to
+95
| ~GraphReinitGuard() { | ||
| if (!dismissed) { | ||
| helper.reinitialize(config, sidePacketMaps); |
Comment on lines
+110
to
+114
| void GraphHelper::reinitialize(const ::mediapipe::CalculatorGraphConfig& config, const GraphSidePackets& sidePacketMaps) { | ||
| SPDLOG_DEBUG("Reinitializing graph after error"); | ||
| // Tear down the old graph (best-effort, errors expected since graph is in bad state) | ||
| if (this->graph) { | ||
| auto absStatus = this->graph->CloseAllPacketSources(); |
Comment on lines
+128
to
+132
| auto absStatus = graph->Initialize(config); | ||
| if (!absStatus.ok()) { | ||
| SPDLOG_ERROR("Graph reinitialize: Initialize failed: {}", absStatus.ToString()); | ||
| graph.reset(); | ||
| return; |
dkalinowski
reviewed
May 25, 2026
| graph->Cancel(); | ||
| } | ||
|
|
||
| void GraphHelper::reinitialize(const ::mediapipe::CalculatorGraphConfig& config, const GraphSidePackets& sidePacketMaps) { |
Collaborator
There was a problem hiding this comment.
Unit test that will ensure failed graphs are reusable is missing
Comment on lines
+93
to
+97
| ~GraphReinitGuard() { | ||
| if (!dismissed) { | ||
| helper.reinitialize(config, sidePacketMaps); | ||
| } | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
In that case we could only log error anyway. Will add that.
Comment on lines
+128
to
+133
| auto absStatus = graph->Initialize(config); | ||
| if (!absStatus.ok()) { | ||
| SPDLOG_ERROR("Graph reinitialize: Initialize failed: {}", absStatus.ToString()); | ||
| graph.reset(); | ||
| return; | ||
| } |
Comment on lines
138
to
142
| if (!absStatus.ok()) { | ||
| SPDLOG_DEBUG("Graph queue WaitUntilDone error: {}", absStatus.ToString()); | ||
| SPDLOG_ERROR("Graph reinitialize: ObserveOutputStream failed: {}", absStatus.ToString()); | ||
| graph.reset(); | ||
| return; | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
Will remove graph.reset.
Comment on lines
+157
to
+162
| absStatus = graph->StartRun(inputSidePackets); | ||
| if (!absStatus.ok()) { | ||
| SPDLOG_ERROR("Graph reinitialize: StartRun failed: {}", absStatus.ToString()); | ||
| graph.reset(); | ||
| return; | ||
| } |
atobiszei
commented
May 29, 2026
| #endif | ||
| if (log_level == "DEBUG" || log_level == "TRACE") { | ||
| #ifdef _WIN32 | ||
| _putenv_s("OPENVINO_LOG_LEVEL", "4"); |
Collaborator
Author
There was a problem hiding this comment.
Not using guard utils since we dont want cyclic dependency on logging.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.