Skip to content

Fully clean up buffers/memory resources in tests - #2243

Merged
seberg merged 1 commit into
NVIDIA:mainfrom
seberg:ft-testing-mr-cleanup
Jun 23, 2026
Merged

Fully clean up buffers/memory resources in tests#2243
seberg merged 1 commit into
NVIDIA:mainfrom
seberg:ft-testing-mr-cleanup

Conversation

@seberg

@sebergseberg commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Maybe the final (small) split-out for the free-threading test PR.

Add explicit sync points and notes around mr.close() making sure that there is no MR shutdown with pending async buffer freeing. These can currently cause occasional dead-locks when we have many MRs in parallel. I believe this is a small upstream issue (625181).

@seberg
Fully clean up buffers/memory resources in tests
b478199
Add explicit sync points and notes around `mr.close()` making sure
that there is no MR shutdown with pending async buffer freeing.
These can currently cause occasional dead-locks when we have many
MRs in parallel.
Signed-off-by: Sebastian Berg <sebastianb@nvidia.com>
@sebergseberg added this to the cuda.core v1.1.0 milestone Jun 23, 2026
@sebergseberg self-assigned this Jun 23, 2026
@sebergseberg added P1 Medium priority - Should do test Improvements or additions to tests cuda.core Everything related to the cuda.core module labels Jun 23, 2026
@copy-pr-bot

copy-pr-botBot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

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.

@mdboom

mdboom commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Somewhat adjacent issue: #1719

mdboom
mdboom approved these changes Jun 23, 2026
View reviewed changes

@mdboommdboom left a comment

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.

I see no reason not to merge this.

As an aside, though... How did you find all of these and is there a way to enforce that this kind of thing doesn't creep back in in the future?

@seberg

seberg commented Jun 23, 2026

Copy link
Copy Markdown
ContributorAuthor

is there a way to enforce that this kind of thing doesn't creep back in in the future?

That'll be the full changes at #2194 to run with pytest-run-parallel, I was just splitting things out since it seemed a bit easier to move forward.

To be fair, cleanup eventually happens so with single threaded (no pytest-run-parallel) I am not sure this can fail (at least in practice).

/ok to test b478199

@seberg

seberg commented Jun 23, 2026

Copy link
Copy Markdown
ContributorAuthor

/ok to test b478199

@github-actions

This comment has been minimized.

Sign in to view
@seberg
seberg merged commit ac3b15d into NVIDIA:mainJun 23, 2026
114 checks passed
@seberg
seberg deleted the ft-testing-mr-cleanup branch June 23, 2026 16:09
@github-actions

This comment has been minimized.

Sign in to view
1 similar comment
@github-actions

github-actionsBot commented Jun 24, 2026

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

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

Reviewers

@mdboommdboommdboom approved these changes

Assignees

@sebergseberg

Labels

cuda.coreEverything related to the cuda.core moduleP1Medium priority - Should dotestImprovements or additions to tests

Projects

None yet

Milestone

cuda.core v1.1.0

Development

Successfully merging this pull request may close these issues.

2 participants

@seberg@mdboom