Skip to content

Conversation

kthui
Copy link
Contributor

@kthui kthui commented Dec 11, 2024

What does the PR do?

RCA: When deleting NumPy arrays in C++, GIL must be held.

Fixes:

  • Ensure GIL is held when deleting NumPy arrays behind tensor objects - fix RCA.
  • Update shutdown step to ensure Python objects are actually deleted during destructor execution.

Checklist

  • PR title reflects the change and is of format <commit_type>: <Title>
  • Changes are described in the pull request.
  • Related issues are referenced.
  • Populated github labels field
  • Added test plan and verified test passes.
  • Verified that the PR passes existing CI.
  • Verified copyright is correct on all changed files.
  • Added succinct git squash message before merging ref.
  • All template sections are filled out.
  • Optional: Additional screenshots for behavior/output changes with before/after.

Commit Type:

Check the conventional commit type
box here and add the label to the github PR.

  • build
  • ci
  • docs
  • feat
  • fix
  • perf
  • refactor
  • revert
  • style
  • test

Related PRs:

triton-inference-server/server#7866

Where should the reviewer start?

Look at this PR first, and then move on to the related PR for the new test.

Test plan:

A new test is added to the related PR that enhances requested output testing.

  • CI Pipeline ID: 21321591

Caveats:

N/A

Background

N/A

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

N/A

@kthui kthui added the PR: fix A bug fix label Dec 11, 2024
@kthui kthui self-assigned this Dec 11, 2024
@kthui kthui requested review from rmccorm4 and krishung5 December 11, 2024 18:19
@kthui kthui marked this pull request as ready for review December 11, 2024 18:19
@kthui kthui requested a review from krishung5 December 11, 2024 19:54
Copy link
Contributor

@krishung5 krishung5 left a comment

Choose a reason for hiding this comment

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

Nice find, Jacky!

@kthui kthui merged commit b771f4f into main Dec 11, 2024
3 checks passed
@kthui kthui deleted the jacky-py-obj-del-gil branch December 11, 2024 22:06
kthui added a commit that referenced this pull request Dec 11, 2024
* fix: Hold GIL when deleting numpy array

* chore: setting py obj to None may not destruct the object
mc-nv pushed a commit that referenced this pull request Dec 12, 2024
* fix: Hold GIL when deleting numpy array

* chore: setting py obj to None may not destruct the object
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: fix A bug fix
Development

Successfully merging this pull request may close these issues.

3 participants