Skip to content

x/sys: stdio handle inheritance getting changed after dynamic loading of a DLL #52498

Closed

Description

@aayush13890

This issue was first reported against golang/go repo and was addressed in Go 1.17.
Issue: #44876
Fix: 4c8f48e

But this change is not part of golang/sys code. Changes in the above commit need to be made in golang/sys repo as well. Thanks!

Activity

  1. added this to the Unreleased milestone on Apr 22, 2022
  2. added
    WaitingForInfoIssue is not actionable because of missing required information, which needs to be provided.
    on Apr 22, 2022
  3. thanm commented on Apr 22, 2022

    @thanm
    Contributor

    Sounds like a good change to make.

    Want to send a CL?

  4. alexbrainman commented on Apr 23, 2022

    @alexbrainman
    Member

    I doubt anyone uses golang.org/x/sys/windows.Stdin. People use either os.Stdin or syscall.Stdin.

    But sure. We should change golang.org/x/sys/windows code too.

    @aayush13890 do you want to send a change?

    Here is how to contribute

    https://go.dev/doc/contribute

    Alex

  5. rkennedy commented on Apr 23, 2022

    @rkennedy

    I agree that few people actually use that Stdin. However, the init function will still always initialize those variables, which has the process-wide side effect of clearing the inheritance flags from the standard handles, so it's definitely a bug that needs fixing.

    On the other hand, if it's really the case that nobody uses the identifiers from x/sys, then perhaps the best action would be to remove the declarations entirely?

  6. aayush13890 commented on Apr 25, 2022

    @aayush13890
    Author

    @alexbrainman @thanm any suggestions on the @rkennedy 's question above? I will go through the steps to contribute in the meanwhile. Thanks!

  7. alexbrainman commented on Apr 26, 2022

    @alexbrainman
    Member

    @alexbrainman @thanm any suggestions on the @rkennedy 's question above?

    @aayush13890 if you refer to

    the best action would be to remove the declarations entirely

    I don't think we can remove the declarations. We always try not to break people code. So even if one person is using this code, we should try and fix them.

    Alex

  8. gopherbot commented on Apr 27, 2022

    @gopherbot
    Contributor

    Change https://go.dev/cl/402714 mentions this issue: windows: do not change stdio handle inheritance

  9. aayush13890 commented on Apr 27, 2022

    @aayush13890
    Author

    @alexbrainman @rkennedy @thanm I have submitted the CL. I am making a change to this repo for the first time. Please let me know if I am missing anything. Also, how do I build the code and run all the tests inside
    sys/windows folder in my source code (https://github.com/golang/sys/tree/master/windows)?

    I tried running:

    -bash-4.2#pwd
    /root/Aayush/go-repos/go-sys/sys

    -bash-4.2# ls
    AUTHORS codereview.cfg CONTRIBUTING.md CONTRIBUTORS cpu execabs go.mod internal LICENSE PATENTS plan9 README.md unix windows

    -bash-4.2# go test ./...
    warning: GOPATH set to GOROOT (/usr/local/go) has no effect
    ok golang.org/x/sys/cpu (cached)
    ok golang.org/x/sys/execabs 0.016s
    ok golang.org/x/sys/internal/unsafeheader (cached)
    ok golang.org/x/sys/unix 0.612s
    ok golang.org/x/sys/unix/internal/mkmerge (cached)
    ? golang.org/x/sys/windows/mkwinsyscall [no test files]

  10. locked and limited conversation to collaborators on Apr 29, 2023
  11. added a commit that references this issue on Jun 7, 2024
    84afa8d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    FrozenDueToAgeWaitingForInfoIssue is not actionable because of missing required information, which needs to be provided.

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions