Skip to content

Conversation

@mdboom
Copy link
Contributor

@mdboom mdboom commented Nov 17, 2025

This updates external (nvvm, nvjitlink and cufile) based on a few bugfixes. This wraps up 3 separate changes:

  • Fix a race condition where the init function could be called multiple times (increasing startup times)

  • Adds cdef _get_ptr to class wrappers as a way to get a pointer to the underlying buffer without making a Python call (this is largely in support of future work to refactor how the buffer is allocated, in support of the future NVML bindings) -- In this case, there are no functions here that make use of _get_ptr, but since they aren't exposed, Cython generates a static C function which the linker will remove because no one uses it. For example:

static intptr_t __pyx_f_4cuda_8bindings_6cufile_13_py_anon_pod1__get_ptr(struct __pyx_obj_4cuda_8bindings_6cufile__py_anon_pod1 *__pyx_v_self);
  • Fixes a bug where string buffers that have invalid UTF-8 in "garbage" memory after the null-terminator character would raise a UnicodeDecodeError.

@mdboom mdboom requested a review from leofang November 17, 2025 13:17
@copy-pr-bot
Copy link
Contributor

copy-pr-bot bot commented Nov 17, 2025

Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@mdboom
Copy link
Contributor Author

mdboom commented Nov 17, 2025

/ok to test

@mdboom mdboom changed the title Update cybind-generated bindings Update external library bindings Nov 17, 2025
@github-actions

This comment has been minimized.

@leofang
Copy link
Member

leofang commented Nov 17, 2025

Q: Should we turn this function into a no-op (or remove it) to confirm the string fix?

def safe_decode_string(raw_value):
"""Safely decode a string value from ctypes buffer."""
# Find null terminator if present
null_pos = raw_value.find(b"\x00")
if null_pos != -1:
raw_value = raw_value[:null_pos]
# Decode with error handling
try:
return raw_value.decode("utf-8", errors="ignore")
except UnicodeDecodeError:
# If UTF-8 fails, try to decode as bytes
return str(raw_value)

leofang
leofang previously approved these changes Nov 17, 2025
@leofang leofang added bug Something isn't working P1 Medium priority - Should do cuda.bindings Everything related to the cuda.bindings module enhancement Any code-related improvements labels Nov 17, 2025
@leofang leofang added this to the cuda-python 13-next, 12-next milestone Nov 17, 2025
@leofang leofang added the to-be-backported Trigger the bot to raise a backport PR upon merge label Nov 17, 2025
@mdboom
Copy link
Contributor Author

mdboom commented Nov 17, 2025

@leofang: WRT the "to-be-backported" label... do we usually backport these non-runtime/non-driver changes?

Q: Should we turn this function into a no-op (or remove it) to confirm the string fix?

def safe_decode_string(raw_value):
"""Safely decode a string value from ctypes buffer."""
# Find null terminator if present
null_pos = raw_value.find(b"\x00")
if null_pos != -1:
raw_value = raw_value[:null_pos]
# Decode with error handling
try:
return raw_value.decode("utf-8", errors="ignore")
except UnicodeDecodeError:
# If UTF-8 fails, try to decode as bytes
return str(raw_value)

Yeah, I was just thinking the same thing.

@mdboom
Copy link
Contributor Author

mdboom commented Nov 17, 2025

/ok to test

@leofang
Copy link
Member

leofang commented Nov 17, 2025

WRT the "to-be-backported" label... do we usually backport these non-runtime/non-driver changes?

Yes. We backport all bug fixes and enhancements to the 12.9 branch. For this PR since the diff is simple (not touching GHA files or build system), the bot can automatically backport it after the PR is merged.

@mdboom
Copy link
Contributor Author

mdboom commented Nov 17, 2025

WRT the "to-be-backported" label... do we usually backport these non-runtime/non-driver changes?

Yes. We backport all bug fixes and enhancements to the 12.9 branch. For this PR since the diff is simple (not touching GHA files or build system), the bot can automatically backport it after the PR is merged.

Ah got it. That was the source of my confusion.

@leofang
Copy link
Member

leofang commented Nov 17, 2025

Ah got it. That was the source of my confusion.

Hopefully #1199 can help eliminate the confusion 🙂

@mdboom mdboom merged commit 1a118b7 into NVIDIA:main Nov 17, 2025
57 checks passed
@github-actions
Copy link

Backport failed for 12.9.x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 12.9.x
git worktree add -d .worktree/backport-1243-to-12.9.x origin/12.9.x
cd .worktree/backport-1243-to-12.9.x
git switch --create backport-1243-to-12.9.x
git cherry-pick -x 1a118b746f8cad978845fad000572205abc604b1

@github-actions
Copy link

Doc Preview CI
Preview removed because the pull request was closed or merged.

mdboom added a commit to mdboom/cuda-python that referenced this pull request Nov 17, 2025
mdboom added a commit that referenced this pull request Nov 17, 2025
* Backport #1243

* Do a minimal removal of 3.9 support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working cuda.bindings Everything related to the cuda.bindings module enhancement Any code-related improvements P1 Medium priority - Should do to-be-backported Trigger the bot to raise a backport PR upon merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants