Repository navigation
Conversation
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review: Windows goroot process-identity hardening
Solid, well-reasoned change. The processTree abstraction is consistent across the three build variants (newProcessTree/close/rss/kill with matching signatures), the Windows handle lifecycle is correct (handle closed on the error path, idempotent close(), defer tree.close() runs last), and the PID-reuse defense is sound: the root PID is pinned by an independently retained handle, descendant termination re-verifies creation time after OpenProcess, and windowsProcessTree rejects children older than the process now holding their recorded parent PID. Test coverage for these cases is strong.
A few non-blocking notes are inline. All are minor; nothing here blocks merge.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
LLGo WebAssembly build benchmarks
WebAssembly output sizes
LLGo WebAssembly build measurements
Compared with |
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Timer runtime benchmarks
Compared with |
cpunion
changed the title
cpunion
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Windows GOROOT tests could terminate an unrelated process after PID reuse, or hang during DLL cleanup after Go main had finished. Track creation-time identities and retain the root handle through cleanup; use immediate process termination for Go exit, with C streams flushed before normal main return. Fixes #2746.
Preserve Windows
Pathentries, reuse the initialized ARM64 MSYS2 installation, and scope macOS RSS queries to the complete target process group instead of unrelated processes. Avoid the Darwin GDB fork/exec startup hang by spawning the same inferior suspended and attaching native GDB before continuing. Debugger assertions, execution timeouts and RSS limits remain unchanged.Validation: runner and entry-generation tests pass; 12 Windows C target builds pass. Native Windows acceptance passes 96 repeated original
gcgortexecutions across Go 1.26/1.27, 12 additional build/run comparisons, 96 concurrent GC/exit subprocesses, GC/nogc runtime fixtures, and redirected C printf output. The RSS regression covers both the direct child and its descendant. The Darwin startup helper passes 24 complete GDB integrations across two Intel runners, following the LLDB acceptance suite.Go exit skips DLL process-detach callbacks, following Microsoft's guidance for unknown thread state.
os.Exitretains immediate Go exit behavior; C code needing buffered output at that call must flush it explicitly.