Skip to content

Fix sycl::queue leak in usm_ndarray get_queue()/get_device() #3042

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vlad-perevezentsev wants to merge 3 commits into
base: master
Choose a base branch
from
+12 −2
Open

Fix sycl::queue leak in usm_ndarray get_queue()/get_device() #3042

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cab5283
Fix queue copy leak in dpnp4pybind11 get_queue()/get_device()
vlad-perevezentsev Aug 25, 2026
2bd7421
Add gh-3042 to changelog
vlad-perevezentsev Aug 25, 2026
85e7468
Merge remote-tracking branch 'origin/master' into fix_queue_leak
vlad-perevezentsev Aug 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ This release is compatible with NumPy 2.5.
* Fixed `dpnp.repeat` raising an unclear `TypeError` for a nested sequence of `repeats` [#3024](https://github.com/IntelPython/dpnp/pull/3024)
* Fixed `dpnp.ndarray.view` ignoring the USM element offset of a sliced array, which also caused `dpnp.einsum` to silently return wrong results for a single sliced operand with no summed index [#3037](https://github.com/IntelPython/dpnp/pull/3037)
* Fixed `dpnp.all` and `dpnp.any` aborting when reducing over an empty axis (e.g. an array with a zero-length dimension) [#3021](https://github.com/IntelPython/dpnp/pull/3021)
* Fixed a per-call `sycl::queue` leak in `usm_ndarray::get_queue()`/`get_device()` [#3042](https://github.com/IntelPython/dpnp/pull/3042)

### Security

Expand Down
13 changes: 11 additions & 2 deletions dpnp/include/dpnp4pybind11.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,17 +489,26 @@ class usm_ndarray : public py::object
PyUSMArrayObject *raw_ar = usm_array_ptr();

auto const &api = detail::dpnp_capi::get();
// UsmNDArray_GetQueueRef_ returns an owning copy (DPCTLQueue_Copy,

@antonwolfy antonwolfy Aug 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use a shorter comment?

// i.e. `new sycl::queue`); wrap it in a unique_ptr to avoid leaking a
// queue per call.
DPCTLSyclQueueRef QRef = api.UsmNDArray_GetQueueRef_(raw_ar);

@antonwolfy antonwolfy Aug 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QRef might be NULL on OOM inside DPCTLQueue_Copy, while below dereferencing of NULL is UB.
It seems legacy and can be done in the follow-up PR.

return *(reinterpret_cast<sycl::queue *>(QRef));
std::unique_ptr<sycl::queue> q_ptr{

@antonwolfy antonwolfy Aug 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One point worth considering: freeing via delete (the unique_ptr default deleter) relies on an undocumented implementation detail rather than dpctl's documented ownership contract.

  • DPCTLSyclQueueRef is documented as an opaque pointer, and DPCTLQueue_Copy is annotated __dpctl_give, whose contract is: "the caller now owns the object … to free it, use it exactly once as a value for a __dpctl_take argument" — i.e. DPCTLQueue_Delete.
  • delete happens to be equivalent today only because DPCTLQueue_Delete is literally delete reinterpret_cast<sycl::queue*>(QRef) and dpnp/dpctl share the same runtime/allocator. If dpctl ever changed how it allocates the copy (pool/custom allocator), raw delete would break while DPCTLQueue_Delete would not.

Non-blocking, but to honor the documented contract we could use a unique_ptr with a custom deleter.

@antonwolfy antonwolfy Aug 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A cleaner option that honors the contract and avoids the link dependency: add a UsmNDArray_RemoveQueueRef capi export that binds DPCTLQueue_Delete, and route the free through the capsule function pointer — mirroring how UsmNDArray_GetQueueRef_ already works, so no consumer needs to link libDPCTLSyclInterface.

dpnp/tensor/_usmarray.pyx:

cdef api void UsmNDArray_RemoveQueueRef(c_dpctl.DPCTLSyclQueueRef QRef):
    """Delete a DPCTLSyclQueueRef previously returned by UsmNDArray_GetQueueRef"""
    c_dpctl.DPCTLQueue_Delete(QRef)   # safe on NULL

dpnp/include/dpnp4pybind11.hpp — add the struct member / ctor-init / assignment mirroring UsmNDArray_GetQueueRef_, then:

DPCTLSyclQueueRef QRef = api.UsmNDArray_GetQueueRef_(raw_ar);
auto qref_deleter = [](sycl::queue *q) {
    detail::dpnp_capi::get().UsmNDArray_RemoveQueueRef_(
        reinterpret_cast<DPCTLSyclQueueRef>(q));
};
std::unique_ptr<sycl::queue, decltype(qref_deleter)> q_ptr{
    reinterpret_cast<sycl::queue *>(QRef), qref_deleter};
return *q_ptr;

This keeps allocation and deallocation on dpctl's side of the ABI and gives a symmetric Get/Remove pair (the capi test would be the natural place for a round-trip check).

That change is non-blocking for the PR — the current delete is provably equivalent under the supported build. So can be also done in the follow-up.

reinterpret_cast<sycl::queue *>(QRef)};
return *q_ptr;
}

sycl::device get_device() const
{
PyUSMArrayObject *raw_ar = usm_array_ptr();

auto const &api = detail::dpnp_capi::get();
// UsmNDArray_GetQueueRef_ returns an owning copy; wrap it in a
// unique_ptr to avoid leaking a queue per call.
DPCTLSyclQueueRef QRef = api.UsmNDArray_GetQueueRef_(raw_ar);
return reinterpret_cast<sycl::queue *>(QRef)->get_device();
std::unique_ptr<sycl::queue> q_ptr{
reinterpret_cast<sycl::queue *>(QRef)};
return q_ptr->get_device();
}

int get_typenum() const
Expand Down
Loading