[SYCL] Release deferred resources after event and queue waits

[ldorau]

Deferred buffers, images and auxiliary resources can otherwise remain
alive until later scheduler activity or runtime shutdown. Release completed
resources after event waits, and sweep deferred resources after queue waits
so completed resources attached to other events are released too.

Use event-indexed auxiliary cleanup for targeted event waits, keeping
unrelated incomplete events from being polled. Destroy extracted resources
outside their mutex to allow destructor-triggered nested waits. Recheck
resources after slow-path waits and avoid reserving space for the entire
auxiliary-resource map when only a few entries are complete.

Publish the Scheduler pointer atomically and guard its lifetime during
wait-triggered cleanup. Preserve cleanup in wait tracing and avoid waiting
for scheduler-access guards during Windows process termination.

Add scheduler tests for nested waits, slow-path registration, targeted
event polling, scheduler lifetime synchronization, and queue-wait cleanup.

This is an improvement in resource lifetime and cleanup behavior.
It releases completed deferred resources after waits
instead of leaving them alive until later scheduler activity
or runtime shutdown. That can reduce memory retained
by long-running applications and avoid resources -
and the queues or contexts they keep alive - persisting through shutdown.

Fixes: #22233
Fixes: #20852

[ldorau]

Please review @slawekptak

[ldorau]

All Windows CI jobs passed. Now enabling all CI jobs ...

[Copilot]

Copilot review overview

🟡 Changes recommended

The lock-free scheduler lookup introduces a data race and possible dangling-pointer access.

Review effort: Balanced
Findings: 1

Open (1)

• Unsynchronized scheduler pointer access causes races and dangling references · New

What changed in this PR

Proactively releases completed SYCL scheduler resources after event::wait() to avoid deferred lifetime leaks.

Changes:

• Tracks deferred resources with an atomic counter.
• Triggers non-blocking cleanup after applicable event waits.
• Adds unit coverage and re-enables Windows leak tests.

File	Description
sycl/​source/​detail/​event_impl.cpp	Initiates deferred cleanup after waits.
sycl/​source/​detail/​global_handler.hpp	Declares scheduler lookup API.
sycl/​source/​detail/​global_handler.cpp	Implements scheduler lookup.
sycl/​source/​detail/​scheduler/​scheduler.hpp	Adds deferred-resource tracking.
sycl/​source/​detail/​scheduler/​scheduler.cpp	Maintains the resource counter.
sycl/​unittests/​scheduler/​GraphCleanup.cpp	Tests wait-triggered cleanup.
sycl/​test-e2e/​Reduction/​reduction_resource_leak_usm.cpp	Re-enables Windows coverage.
sycl/​test-e2e/​Reduction/​reduction_resource_leak_dw.cpp	Re-enables Windows coverage.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[iclsrc]

Reviewed the proactive deferred-resource release in event_impl::wait() plus the supporting Scheduler::MDeferredResourcesCount atomic counter and GlobalHandler::getSchedulerIfAlive() accessor. I traced every increment/decrement site for the counter (cleanupCommands, deferMemObjRelease, cleanupDeferredMemObjects, takeAuxiliaryResources, registerAuxiliaryResources, cleanupAuxiliaryResources) against each insertion/removal from MDeferredCleanupCommands, MDeferredMemObjRelease, and MAuxiliaryResources, including the takeAuxiliaryResources merge-into-existing-entry edge case, and the bookkeeping is balanced throughout. The WaitedViaScheduler short-circuit in event_impl::wait() correctly avoids a redundant release pass when Scheduler::waitForEvent() already performed one via cleanupCommands. getSchedulerIfAlive() mirrors the existing lock-free isSchedulerAlive() pattern, so no new thread-safety concern is introduced. Relaxed memory ordering on the counter is appropriate given it's documented as a best-effort hint, with actual releases still happening under the respective mutexes. The new AuxiliaryResourcesReleasedOnWait unit test and the re-enabled Windows E2E tests (reduction_resource_leak_dw.cpp, reduction_resource_leak_usm.cpp) are consistent with and validate the stated goal. No correctness, security, or test-coverage issues found.

[Copilot]

Copilot review overview

🟡 Changes recommended

The cleanup path introduces re-entrant deadlock and Scheduler lifetime risks, while also ending XPTI tracing prematurely.

Review effort: Balanced
Findings: 3

Open (3)

• Deadlock from holding auxiliary-resource lock during blocking wait · New
• Atomic scheduler pointer permits use-after-free during shutdown · New
• Unsynchronized scheduler pointer access causes races and dangling references

Previously missed (1)

In code that hasn't changed since last review

Wait-end trace emitted before resource release completes

sycl/​source/​detail/​event_impl.cpp:429

trace_wait_end is emitted before this newly added work, so XPTI reports event::wait() as finished while releaseResources() is still running. Contention or cleanup cost will therefore be omitted from wait profiles and the trace's call ordering can precede the actual return. Move the instrumentation epilog after this cleanup, as queue_impl::wait() does after all of its wait work.

[Copilot]

Copilot review overview

🟡 Changes recommended

Windows shutdown can spin indefinitely when a terminated thread leaves a scheduler access count outstanding.

Review effort: Balanced
Findings: 2

Open (2)

• Avoid shutdown hang from abandoned scheduler accesses on Windows termination · New
• Deadlock from holding auxiliary-resource lock during blocking wait

Resolved since last review (2)

• Atomic scheduler pointer permits use-after-free during shutdown
• Unsynchronized scheduler pointer access causes races and dangling references

[Copilot]

Copilot review overview

🟡 Changes recommended

The new scheduler teardown synchronization lacks deterministic concurrency coverage.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Deadlock from holding auxiliary-resource lock during blocking wait
• Add multithreaded coverage for scheduler access lifetime protocol · New

Resolved since last review (1)

• Avoid shutdown hang from abandoned scheduler accesses on Windows termination

[Copilot]

Copilot review overview

🟡 Changes recommended

Auxiliary-resource destruction can deadlock during nested waits, and slow-path waits can still leave newly deferred resources unreleased.

Review effort: Balanced
Findings: 2 · 1

Open (3)

• Destroy auxiliary resources outside the mutex · New
• Deadlock from holding auxiliary-resource lock during blocking wait
• Add multithreaded coverage for scheduler access lifetime protocol

Previously missed (1)

In code that hasn't changed since last review

Slow-path wait can retain resources after cleanup

sycl/​source/​detail/​event_impl.cpp:410

The scheduler slow path is not guaranteed to leave the deferred sets empty. waitForEvent() calls cleanupCommands(), which drains deferred resources before deleting the commands; those deletions can themselves defer new memory objects (the post-cleanup case documented in Scheduler::releaseResources()). Marking every slow-path wait as already cleaned therefore skips the only final pass and can still retain resources after the application's last wait. Check the counter after both wait paths; it remains lock-free and avoids the extra pass when cleanup really completed.

[Copilot]

Copilot review overview

🟡 Changes recommended

Blocking auxiliary cleanup can still release resources while holding its mutex, allowing nested waits to deadlock.

Review effort: Balanced
Findings: 2 · 1

Open (3)

• Release completed resources outside mutex to prevent reentrant deadlock · New
• Deadlock from holding auxiliary-resource lock during blocking wait
• Add multithreaded coverage for scheduler access lifetime protocol

Resolved since last review (1)

• Destroy auxiliary resources outside the mutex

[Copilot]

Copilot review overview

🔵 Needs a closer look

Scheduler lifetime synchronization and Windows shutdown behavior are concurrency-sensitive and warrant final human review.

Review effort: Balanced
Findings: None

[iclsrc]

Round-2 review: Re-examined the SchedulerAccess RAII guard (GlobalHandler), the atomic generation/counter bookkeeping added to Scheduler (MDeferredResourcesCount, MAuxiliaryResourcesGeneration), and the Windows shutdown-ordering fix (ProcessTerminating / stopSchedulerAccess). All increment/decrement pairs across cleanupCommands, cleanupDeferredMemObjects, registerAuxiliaryResources, takeAuxiliaryResources, and both blocking/non-blocking cleanupAuxiliaryResources paths (including the exception-retry path) are consistent. The Windows skip-wait path during process termination is correctly scoped to the case where other threads are already OS-terminated, matching the existing CanJoinThreads semantics in this file. No Critical or Important issues found in this round.

[slawekptak]

With the additional logic added to event::wait(), could you please make sure there is no performance impact for the fast path benchmarks we have (probably it makes sense to focus on the tests which include the completion path)?

[ldorau]

SYCL benchmarks started: https://github.com/intel/llvm/actions/runs/37304711992

[ldorau]

SYCL benchmarks started: https://github.com/intel/llvm/actions/runs/37304711992

The results are available at https://intel.github.io/llvm/benchmarks/?runs=Baseline_BMG_L0v2%2CPR_23344_BMG_L0v2

[sys-ce-bb]

@intel/llvm-gatekeepers please consider merging

[ldorau]

Could you review this PR? @uditagarwal97 @KseniyaTikhomirova @sergey-semenov

[ldorau]

This PR is marked as a draft, because it is waiting for review from: @sergey-semenov

[sergey-semenov]

The approach LGTM, just a few non-blocking nitpicks

[sergey-semenov]

Suggested change

	cleanupCommands(ToCleanUp, false);
	cleanupCommands(ToCleanUp, /*ScanAuxiliaryResources*/false);

[ldorau]

Done

[sergey-semenov]

Same here

[ldorau]

Done

[sergey-semenov]

This belongs with the rest of the standard library includes below.

[ldorau]

Done

[ldorau]

The approach LGTM, just a few non-blocking nitpicks

I have applied them all. Thank you very much!

[ldorau]

The two tests which this patch was supposed to fix failed again in the same way - converting to draft ...

[ldorau]

The two tests which this patch was supposed to fix failed again in the same way - converting to draft ...

It was caused by too old version of L0 loader on Windows - see d800470.

[ldorau]

Please merge @intel/llvm-gatekeepers