pi_event objects are not destroyed by the runtime

[nyalloc]

It has been observed in the PI CUDA backend that pi_context objects, pi_queue objects and pi_event objects can be leaked by the runtime.

In the PI CUDA backend, pi_context and pi_queue objects are retained by pi_event objects. The runtime creates many new pi_event objects without releasing them.

llvm/sycl/source/detail/scheduler/commands.cpp

Line 1779 in 3f21371

Plugin.call<PiApiKind::piEnqueueEventsWait>(MQueue->getHandleRef(), 0, nullptr, &Event);

llvm/sycl/source/detail/memory_manager.cpp

Line 480 in 3f21371

Plugin.call<PiApiKind::piEnqueueMemBufferMap>(

These functions return new pi_event objects as an output parameter. These new objects are never released. This is likely to affect all piEnqueue... functions.

The only call to piEventRelease in the runtime source outside of the PI CUDA backend is in the destructor of the event_impl class.

llvm/sycl/source/detail/event_impl.cpp

Line 49 in 092367b

getPlugin().call<PiApiKind::piEventRelease>(MEvent);

event_impl's destructor never calls piEventRelease in any of our tests. The lifetime of event_impl objects are managed through std::shared_ptr, so it is possible that a circular reference has occurred, or that a stray copy of the shared_ptr is preventing the event object from being released.

To ensure PI API can be used correctly across all backends, all pi_event objects should be explicitly released when they are no longer needed. This will require some investigation into pi_event use through the runtime to determine if all events created are appropriately released.

[sergey-semenov]

The first iteration of the patch that addresses the issue should be up for review on Tuesday.

[sergey-semenov]

I thought I found a reproducer for this behaviour, but on further investigation this turned out not to be the case. In stark contrast to "event_impl's destructor never calls piEventRelease in any of our tests", all the tests I inspected had a piEventRelease call for each piEventCreate/piEventRetain/piEnqueue*... call (outputs for two tests with SYCL_PI_TRACE=1 attached: accessor.txt DataMovement.txt).

Moreover, after inspecting the code for event-producing/retaining PI calls, I haven't discovered any issues: the runtime currently stores a shared_ptr<event_impl> for such events in the command graph and the queue, and they are released once the related command nodes are cleaned up (at a host-device sync point or during memory object destruction) and the event is removed from the queue (via a call to queue::wait() or during queue destruction).

Could you please provide a reproducer for this problem?

[sergey-semenov]

and they are released once the related command nodes are cleaned up ... and the event is removed from the queue

As a side note, here's a PR that changes that behaviour in order to release events earlier: #1561

[romanovvlad]

@StuartDAdams ping

[nyalloc]

Hello! I'll try to get a reproducer ASAP.

[nyalloc]

This was first observed when running OneMKL's test_main_ct test executable. The leaks have been reproduced using the newest LLVM and OneMKL available. This test runs on the PI CUDA backend.

Intel LLVM: cece82e
OneMKL: 751472666ba884397837bbc61961bbadd5670bd0

A branch of OneMKL has been created here that highlights the leak. https://github.com/StuartDAdams/oneMKL/tree/stuart/leakRepro

Running the test_main_ct exectable with a debugger and inspecting calls made to the cuda_piConextRelease, cuda_piEventRelease and cuda_piQueueRelease functions, it is clear that pi objects are being retained and never released before the application exits. Most calls to release simply decrement the reference count and return. By the end of the application, it has leaked two pi_context objects, two pi_queue objects, and four pi_event objects.

[sergey-semenov]

I've tried the linked reproducer with the latest build (albeit I hacked my way around installing a reference BLAS implementation, attached the diff just in case: Diff.txt).

I haven't observed any leaks in the test case on the DPCPP runtime side when inspecting the calls to cuda_pi*Release:

• Each of the 4 events in the test are released twice: once in the CUDA PI plugin after a retain in _pi_event::record() and once in the shared_ptr<event_impl> destructor, lowering the pi_event reference count to 0.
• Each of the 2 queues is released 4 times, again, lowering the reference count to 0.
• 2 created contexts are released once, lowering the reference count to 0, and 2 of them are released 6 times (twice during piEventRelease - retained at piEnqueue*, twice during piMemRelease - retained at piMemBufferCreate, once during piQueueRelease - retained during piQueueCreate, and once during ~context_impl()), but there's an extra retain during the call to context::get() from onemkl::cublas::CublasScopedContextHandler::get_handle(), which as far as I can tell is never released, leaving the reference count at 1 at the end of the application. So there's an issue here, but it looks unrelated to the DPCPP runtime.

The PI trace obtained from the test case also seems to confirm that there's no issue on the DPCPP runtime side (attached: trace.txt).

@StuartDAdams Could you please double-check my findings?

[nyalloc]

No problem. Thanks for looking into this. I'll have a look over the behaviour on my side again now.

[nyalloc]

Using the same branches as last time, I can observe each of the four pi_event objects being created through _pi_event::make_native, retained once through cuda_piEventRetain, and released once through cuda_piEventRelease. They are left with a refcount of 1, and with no subsequent releases, they all leak. While the event_impl class destructor is called 6 times, the MEvent member is always null, meaning it never bothers to call piEventRelease.

[nyalloc]

For details on builing OneMKL using Intel LLVM with CUDA, please see this thread.

#1548

[romanovvlad]

@StuartDAdams
Could you please provide:

1. standalone reproducer which does not depend on libraries?
2. logs with pi trace enabled for the oneMKL reproducer?