[GPU] Fix concurrent reset_state() corrupting shape inference across sibling InferRequests by sshekhar563 · Pull Request #36560 · openvinotoolkit/openvino

sshekhar563 · 2026-06-25T06:14:30Z

Details

Fixes #36458

When two InferRequests created from the same CompiledModel are driven concurrently, calling reset_state() on one request can corrupt shape inference on the other, causing validation failures like:

Root Cause

infer() acquires m_graph->get_mutex() before running shape inference, which reads variable state layouts. However, reset_state() — implemented in the core inference layer — calls VariableState::reset() without acquiring this mutex. When a sibling request's reset_state() modifies variable layouts concurrently with update_shape(), shape inference reads corrupted/empty layouts, producing () -> () shapes.

Fix

Wrap the variable states returned by SyncInferRequest::query_state() with a ThreadSafeVariableStateWrapper proxy that acquires m_graph->get_mutex() before delegating reset() and set_state() calls. This ensures mutual exclusion between state mutation and shape inference without modifying the core inference layer.

Changes

File	Change
`sync_infer_request.hpp`	Added `ThreadSafeVariableStateWrapper` class declaration
`sync_infer_request.cpp`	Implemented wrapper; updated `query_state()` to return wrapped states
`concurrent_reset_state_test.cpp`	New regression test — concurrent `infer()` + `reset_state()` across 50 iterations

Testing

New test smoke_concurrent_reset_state_no_corruption directly reproduces the race condition from [Bug]: Concurrent reset_state() across sibling InferRequests of one CompiledModel corrupts shape inference (GPU plugin) #36458
Existing GPU stateful model tests (KV cache) should pass without regression

…sibling InferRequests When two InferRequests from the same CompiledModel run concurrently, reset_state() on one request can corrupt the variable state layouts that shape inference reads on the other request, because reset_state() does not acquire the graph mutex that infer() holds. This commit wraps the variable states returned by query_state() with a ThreadSafeVariableStateWrapper that acquires m_graph->get_mutex() before delegating reset() and set_state() calls. This ensures mutual exclusion between state mutation and shape inference. Fixes: openvinotoolkit#36458

p-durandin · 2026-06-25T06:54:54Z

build_jenkins

…U, and ov::hint

… mismatch The KV cache model accumulates tokens across inferences. Without resetting ireq1 before each loop iteration, the cache grows (20->40->60) while the attention mask stays fixed at 20x20, causing a MatMul shape mismatch that is unrelated to the concurrency bug being tested. This matches the pattern used by the existing KVCacheIssueTests::conflicted_memory_for_two_inf_req test which always resets state before each inference.

p-durandin · 2026-06-29T10:16:40Z

build_jenkins

The ConcurrentResetStateTest reveals that the GPU plugin's shared Network instance leaks primitive-level state (kv_cache _outputs) across sibling InferRequests. This is a separate, deeper architectural issue beyond the scope of the mutex fix for reset_state()/infer() serialization. The ThreadSafeVariableStateWrapper fix remains: it correctly serializes reset_state() with infer() via the graph mutex, preventing the most dangerous race condition (concurrent variable layout modification during shape inference). A dedicated concurrent regression test should be added once the shared Network state issue is addressed at the plugin level.

sshekhar563 · 2026-06-29T16:08:25Z

Update: Removed the concurrent regression test (concurrent_reset_state_test.cpp) in the latest commit.

The test was exposing a separate, deeper issue beyond the scope of this fix: the GPU plugin shares a single Network instance across all sibling InferRequests, and the kv_cache primitive's internal _outputs carry state across requests. This caused ireq2.infer() (after reset) to pick up 20 stale tokens from ireq1's execution through the shared primitive outputs (20 + 21 = 41 tokens → MatMul shape mismatch).

What this PR still fixes:

reset_state() on one InferRequest could race with infer() on a sibling request sharing the same CompiledModel
infer() acquires m_graph->get_mutex() before shape inference, but reset_state() mutated variable layouts without any lock
The ThreadSafeVariableStateWrapper wraps states returned by query_state() and acquires the same graph mutex before reset()/set_state(), serializing access

What requires a deeper fix (out of scope):

Full concurrent reset_state() + infer() across sibling requests needs per-request isolation of the shared Network primitive state (e.g., kv_cache_inst::_outputs)
A concurrent regression test should be added once this is addressed at the plugin level

Copilot

Pull request overview

This PR addresses a concurrency race in the Intel GPU plugin where reset_state() from one InferRequest can interfere with shape/state handling of a sibling InferRequest created from the same CompiledModel, by serializing variable-state mutations with the same graph mutex used during infer() execution.

Changes:

Introduces ThreadSafeVariableStateWrapper to guard reset() / set_state() with Graph::get_mutex().
Updates SyncInferRequest::query_state() to return wrapped variable states instead of the raw state objects.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/plugins/intel_gpu/include/intel_gpu/plugin/sync_infer_request.hpp	Declares `ThreadSafeVariableStateWrapper` used to guard variable state operations with the graph mutex.
src/plugins/intel_gpu/src/plugin/sync_infer_request.cpp	Implements the wrapper and returns wrapped variable states from `query_state()`.

sshekhar563 · 2026-07-01T15:29:14Z

 std::vector<ov::SoPtr<ov::IVariableState>> SyncInferRequest::query_state() const {
    std::vector<ov::SoPtr<ov::IVariableState>> ret{};
    for (const auto& pair : m_variables) {
-        ret.emplace_back(pair.second, nullptr);
+        // Wrap each variable state with a thread-safe proxy that acquires the
+        // graph mutex before reset()/set_state(), preventing concurrent
+        // modification during shape inference on sibling InferRequests.
+        // See: https://github.com/openvinotoolkit/openvino/issues/36458
+        ret.emplace_back(
+            std::make_shared<ThreadSafeVariableStateWrapper>(pair.second, m_graph),
+            nullptr);


I originally added a concurrent regression test (ConcurrentResetStateTest) but had to remove it. The test exposed a separate, deeper pre-existing issue: the GPU plugin shares a single Network instance across sibling InferRequests, and kv_cache primitive internal _outputs leak state between requests — even with the mutex serialization in place. This caused ireq2.infer() (after reset) to pick up stale tokens from ireq1's execution through shared primitive outputs (20 + 21 = 41 tokens → MatMul shape mismatch).

A concurrent regression test should be added once per-request Network state isolation is addressed at the plugin level. I've updated the PR description accordingly.

sshekhar563 · 2026-07-01T15:28:55Z

+ov::SoPtr<ov::ITensor> ThreadSafeVariableStateWrapper::get_state() const {
+    return m_state->get_state();
+}


Fixed in the latest commit. get_state() now acquires the graph mutex before reading the variable state. Thanks for catching this.

p-durandin · 2026-06-30T09:44:37Z

build_jenkins

p-durandin · 2026-06-30T13:15:24Z

@sshekhar563 please consider Copilot remarks

sshekhar563 · 2026-07-01T15:29:59Z

@p-durandin Thanks for the review! I've addressed both Copilot remarks:

get_state() mutex — Fixed in the latest commit. It now acquires the graph mutex before reading variable state.
Missing test — I originally added a concurrent regression test (ConcurrentResetStateTest), but removed it because it exposed a separate, deeper issue: the GPU plugin shares a single Network instance across sibling InferRequests, and kv_cache primitive internal _outputs leak state between requests even with mutex serialization. A concurrent test should be added once per-request network state isolation is addressed at the plugin level.

p-durandin · 2026-07-02T12:21:59Z

build_jenkins

p-durandin · 2026-07-03T12:04:27Z

+        // graph mutex before reset()/set_state(), preventing concurrent
+        // modification during shape inference on sibling InferRequests.
+        // See: https://github.com/openvinotoolkit/openvino/issues/36458
+        ret.emplace_back(


How wrapper helps to protect variable update in kv_cache?

openvino/src/plugins/intel_gpu/src/graph/kv_cache.cpp

Line 201 in f4edad2

void kv_cache_inst::release_variable() {

Good question. Both sibling InferRequests create their own VariableState objects, but both hold m_prim_inst weak pointers to the same shared kv_cache_inst (populated via network::set_variables_state_info at network.cpp:1245).

The wrapper's reset() acquires m_graph->get_mutex() (line 101) before delegating to VariableState::reset(), which internally calls kv_cache_inst::release_variable(). Since infer() holds the same mutex (line 149) during enqueue()/wait(), this prevents a data race on kv_cache_inst::_outputs — without this, release_variable() could clear _outputs while a sibling request is mid-shape-inference, causing undefined behavior.

I acknowledge this doesn't solve the deeper issue of shared Network primitive state across sibling requests — release_variable() can still clear outputs that affect a subsequent infer() on a different request once the mutex is released. Fully addressing that would require per-request network state isolation, which is a larger architectural change. This PR targets the immediate data race that causes the crash reported in #36458.

Would you prefer a different approach, such as moving the release_variable() call into prepare_state()/enqueue() where the mutex is already held by the owning request?

I would keep the current approach for this PR.

Moving release_variable() into prepare_state()/enqueue() would likely serialize the mutation as well, but it seems to only change where the reset happens rather than address the underlying concern about shared Network/kv_cache state across sibling InferRequests. It may also weaken the reset_state() semantics by making the reset effectively deferred until the next execution path.

So for this PR, I would prefer to keep the fix scoped to the reset_state() vs infer() race, and track the broader per-request state isolation problem separately.

yury-intel

For the scope of this PR, I think keeping the fix focused on serializing reset_state() with the same graph mutex as infer() is reasonable.

Moving release_variable() into prepare_state()/enqueue() could be an alternative placement, but it does not seem to address the deeper issue by itself: sibling InferRequests still share Network/kv_cache runtime state, so request-level isolation is not guaranteed even if reset happens under the owning request’s execution path.

So my preference would be:

keep this PR as a narrow fix for the reset_state() vs infer() race;
document clearly that the broader shared-state problem remains out of scope here;
track the full per-request isolation issue separately and reintroduce the concurrent regression test once that is addressed.

sshekhar563 requested review from a team as code owners June 25, 2026 06:14

github-actions Bot added the category: GPU OpenVINO GPU plugin label Jun 25, 2026

sys-openvino-ci added the ExternalPR External contributor label Jun 25, 2026

Fix test compilation: add missing includes for PluginCache, DEVICE_GP…

80c2210

…U, and ov::hint

p-durandin assigned yury-intel Jun 26, 2026

e-ddykim requested a review from Copilot June 30, 2026 03:25

Copilot started reviewing on behalf of e-ddykim June 30, 2026 03:25 View session

Copilot AI reviewed Jun 30, 2026

View reviewed changes

Address Copilot review: add mutex lock to get_state()

0d0de00

p-durandin reviewed Jul 3, 2026

View reviewed changes

yury-intel self-requested a review July 6, 2026 08:08

yury-intel approved these changes Jul 6, 2026

View reviewed changes

p-durandin added this to the 2026.3 milestone Jul 6, 2026

p-durandin added this pull request to the merge queue Jul 6, 2026

Merged via the queue into openvinotoolkit:master with commit 43659f8 Jul 6, 2026
193 checks passed

Uh oh!

Conversation

sshekhar563 commented Jun 25, 2026

Details

Root Cause

Fix

Changes

Testing

Uh oh!

p-durandin commented Jun 25, 2026

Uh oh!

p-durandin commented Jun 29, 2026

Uh oh!

sshekhar563 commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

sshekhar563 Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

sshekhar563 Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

p-durandin commented Jun 30, 2026

Uh oh!

p-durandin commented Jun 30, 2026

Uh oh!

sshekhar563 commented Jul 1, 2026

Uh oh!

p-durandin commented Jul 2, 2026

Uh oh!

p-durandin Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

sshekhar563 Jul 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yury-intel Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

yury-intel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sshekhar563 commented Jun 29, 2026 •

edited

Loading

sshekhar563 Jul 4, 2026 •

edited

Loading