Skip to content

Avoid calls to (*os.File).Fd() when calling ioctl#215

Merged
creack merged 3 commits into
creack:masterfrom
WeidiDeng:unsafe-pointer
Jun 1, 2026
Merged

Avoid calls to (*os.File).Fd() when calling ioctl#215
creack merged 3 commits into
creack:masterfrom
WeidiDeng:unsafe-pointer

Conversation

@WeidiDeng

Copy link
Copy Markdown
Contributor

Like #214 , but use unsafe.Pointer as a parameter to avoid the use of reflect.

@WeidiDeng

Copy link
Copy Markdown
Contributor Author

@creack Do you have time to review this pr? I'd like to use the non blocking operation without manually calling syscall.SetNonblock.

@creack

creack commented Mar 12, 2026

Copy link
Copy Markdown
Owner

Will try to take a look over the weekend but may be the weekend after that.

@WeidiDeng

Copy link
Copy Markdown
Contributor Author

@creack Do you have time to review this pr?

@WeidiDeng

Copy link
Copy Markdown
Contributor Author

I think //go:uintptrescapes directive might need very few changes. Will test soon.

@creack

creack commented Jun 1, 2026

Copy link
Copy Markdown
Owner

Thanks @WeidiDeng — really appreciate you sticking with this and following up on the earlier rounds (#214, the //go:uintptrescapes experiment).

The unsafe.Pointer-through-the-stack approach is the right way to satisfy unsafe.Pointer rule (4): the uintptr(ptr) conversion only happens inside the syscall.Syscall argument list, which is what keeps the referenced object alive across the call. That closes a latent GC hazard that's been in the codebase since well before #167.

Merging now. Will hold tagging/release until I've also added a stress test that exercises the SyscallConn().Control() + concurrent close/fork pattern that motivated the original #177 revert — want a more reproducible guard than -count=100. If anyone hits issues against master, please open a follow-up issue.

@creack creack merged commit 9246436 into creack:master Jun 1, 2026
5 checks passed
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.

2 participants