Skip to content

Handle null Cython close requests#685

Open
fallintoplace wants to merge 1 commit into
rapidsai:mainfrom
fallintoplace:fix/cython-null-close-request
Open

Handle null Cython close requests#685
fallintoplace wants to merge 1 commit into
rapidsai:mainfrom
fallintoplace:fix/cython-null-close-request

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

Summary

  • make UCXEndpoint.close() return None when C++ returns an empty close request
  • guard UCXRequest request-pointer access before dereferencing
  • add a low-level regression for calling endpoint close while close is already in progress

Why

Endpoint::closeRequest() can return nullptr when the endpoint is already closing or closed. Wrapping that empty shared_ptr as UCXRequest lets the Cython destructor and accessors dereference self._request.get() even though it is null.

Returning None keeps repeated close calls idempotent for cleanup paths, while the UCXRequest guard prevents a crash if an empty wrapper is ever constructed by another path.

Validation

  • git diff --check
  • uvx --from 'cython>=3.2.2' cython -3 --cplus python/ucxx/ucxx/_lib/libucxx.pyx -o .cython-check/libucxx.cpp
  • uvx ruff check python/ucxx/ucxx/_lib/tests/test_endpoint.py
  • Tried cmake -S cpp -B cpp/build/local-check -G Ninja -DBUILD_TESTS=OFF -DBUILD_BENCHMARKS=OFF -DBUILD_EXAMPLES=OFF -DUCXX_ENABLE_CCCL=OFF; configure is blocked locally because UCX is not installed (ucxConfig.cmake / ucx-config.cmake not found).

@fallintoplace fallintoplace requested a review from a team as a code owner June 15, 2026 21:49
@copy-pr-bot

copy-pr-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant