Skip to content
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.

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

internal: use runtime.Pinner in PtrGuard #935

Merged
merged 1 commit into from
Oct 23, 2023
Merged

Conversation

ansiwen
Copy link
Collaborator

@ansiwen ansiwen commented Oct 19, 2023

Since Go 1.21 there is a runtime.Pinner API that allows to safely pass structures with embedded Go pointers to C code. In earlier Go version we know that the garbage collector is non-moving, so it is safe to pass Go pointers to C as well. This change adds two implementations of PtrGuard, one for pre 1.21 that is basically a no-op, and one for 1.21+ that uses runtime.Pinner.

Since Go 1.21 there is a runtime.Pinner API that allows to safely
pass structures with embedded Go pointers to C code.  In earlier Go
version we know that the garbage collector is non-moving, so it is
safe to pass Go pointers to C as well. This change adds two
implementations of PtrGuard, one for pre 1.21 that is basically a
no-op, and one for 1.21+ that uses runtime.Pinner.

Signed-off-by: Sven Anderson <[email protected]>
@phlogistonjohn phlogistonjohn added the no-API This PR does not include any changes to the public API of a go-ceph package label Oct 20, 2023
@phlogistonjohn
Copy link
Collaborator

Looks good on first glance. I want to look at it once more with fresh eyes so I have not approved yet, but I expect to approve it some time in the next couple of days.

@@ -1,90 +1,37 @@
//go:build !go1.21
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is more of a learning for me / double check, can you confirm if this will work correctly on go 1.22 and later? I searched for the precise syntax here and only found articles on the general syntax of build tags and the change from "+build" to "go:build". sigh

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

"a term for each Go major release, through the current version: "go1.1" from Go version 1.1 onward, "go1.12" from Go 1.12, and so on."
https://pkg.go.dev/cmd/go#hdr-Build_constraints

Copy link
Collaborator

@phlogistonjohn phlogistonjohn left a comment

Choose a reason for hiding this comment

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

LGTM

@mergify mergify bot merged commit 780f701 into master Oct 23, 2023
15 checks passed
@mergify mergify bot deleted the pr/ansiwen/pinner branch October 23, 2023 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-API This PR does not include any changes to the public API of a go-ceph package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants