Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion internal/stdlib/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ and the syscall package is locked down for the most part.
3. Almost same story as 2, but in this case we'd like to support assigning a [pseudo console](https://docs.microsoft.com/en-us/windows/console/createpseudoconsole)
to a process. There's no exposed way to pass in the pseudo console handle and there's no syscall package support for making one in the first place.


The stdlib packages have been modified to use the x/sys/windows package where needed, removed some unneccesary functionality that we
don't need, as well as added the additions described above.

The fork is based off of go 1.17 with HEAD at a6ff433d6a927e8ad8eaa6828127233296d12ce5.
The fork is from Go 1.17 with HEAD at a6ff433d6a927e8ad8eaa6828127233296d12ce5.
10 changes: 5 additions & 5 deletions internal/stdlib/execenv/execenv_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
package execenv

import (
"syscall"
"unicode/utf16"
"unsafe"

"internal/syscall/windows"
"github.com/Microsoft/hcsshim/internal/stdlib/syscall"
"golang.org/x/sys/windows"
)

// Default will return the default environment
Expand All @@ -25,14 +25,14 @@ import (
// will be sourced from syscall.Environ().
func Default(sys *syscall.SysProcAttr) (env []string, err error) {
if sys == nil || sys.Token == 0 {
return syscall.Environ(), nil
return windows.Environ(), nil

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.

Does this not build without replacing syscall with windows? Just wondering if we can avoid that so as to keep deviation from upstream as little as possible.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No this would build fine, I swapped to Windows as there's a x/sys/Windows replacement/mirror for everything in syscall and technically the syscall package is locked and some of it is deprecated. I can swap to syscall if preferred. @anmaxvl What do you think?

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 misread the original code a bit, I thought it was internal/syscall and we replaced it with windows, turns out it was originally internal/syscall/windows and syscall was imported on its own.
I agree with @ambarve that we should probably keep syscall here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will try and add everything thats touched in and update, I'll ping when done!

}
var block *uint16
err = windows.CreateEnvironmentBlock(&block, windows.Token(sys.Token), false)
err = windows.CreateEnvironmentBlock(&block, sys.Token, false)
if err != nil {
return nil, err
}
defer windows.DestroyEnvironmentBlock(block)
defer windows.DestroyEnvironmentBlock(block) // nolint: errcheck
blockp := uintptr(unsafe.Pointer(block))
for {

Expand Down
19 changes: 9 additions & 10 deletions internal/stdlib/os/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@ package os

import (
"errors"
"internal/testlog"
"os"
"runtime"
"sync"
"sync/atomic"
"syscall"
"time"

"github.com/Microsoft/hcsshim/internal/stdlib/syscall"
"golang.org/x/sys/windows"
)

// ErrProcessDone indicates a Process has finished.
Expand All @@ -20,9 +21,8 @@ var ErrProcessDone = errors.New("os: process already finished")
// Process stores the information about a process created by StartProcess.
type Process struct {
Pid int
handle uintptr // handle is accessed atomically on Windows
isdone uint32 // process has been successfully waited on, non zero if true
sigMu sync.RWMutex // avoid race between wait and signal
Comment thread
anmaxvl marked this conversation as resolved.
Outdated
handle uintptr // handle is accessed atomically on Windows
isdone uint32 // process has been successfully waited on, non zero if true
}

func newProcess(pid int, handle uintptr) *Process {
Expand Down Expand Up @@ -57,7 +57,7 @@ type ProcAttr struct {
// On Unix systems, StartProcess will change these File values
// to blocking mode, which means that SetDeadline will stop working
// and calling Close will not interrupt a Read or Write.
Files []*File
Files []*os.File

// Operating system-specific process creation attributes.
// Note that setting this field means that your program
Expand All @@ -75,10 +75,10 @@ type Signal interface {
}

// Getpid returns the process id of the caller.
func Getpid() int { return syscall.Getpid() }
func Getpid() int { return windows.Getpid() }

// Getppid returns the process id of the caller's parent.
func Getppid() int { return syscall.Getppid() }
func Getppid() int { return windows.Getppid() }

// FindProcess looks for a running process by its pid.
//
Expand All @@ -105,7 +105,6 @@ func FindProcess(pid int) (*Process, error) {
//
// If there is an error, it will be of type *PathError.
func StartProcess(name string, argv []string, attr *ProcAttr) (*Process, error) {
testlog.Open(name)
return startProcess(name, argv, attr)
}

Expand Down
19 changes: 11 additions & 8 deletions internal/stdlib/os/exec/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,18 @@ import (
"bytes"
"context"
"errors"
"internal/syscall/execenv
"io"
"os"
"path/filepath"
"runtime"
"strconv"
"strings"
"sync"
"syscall"

"github.com/Microsoft/hcsshim/internal/stdlib/execenv"
"github.com/Microsoft/hcsshim/internal/stdlib/syscall"

osfork "github.com/Microsoft/hcsshim/internal/stdlib/os"
)

// Error is returned by LookPath when it fails to classify a file as an
Expand Down Expand Up @@ -127,11 +130,11 @@ type Cmd struct {
SysProcAttr *syscall.SysProcAttr

// Process is the underlying process, once started.
Process *os.Process
Process *osfork.Process

// ProcessState contains information about an exited process,
// available after a call to Wait or Run.
ProcessState *os.ProcessState
ProcessState *osfork.ProcessState

ctx context.Context // nil means none
lookPathErr error // LookPath error, if any.
Expand Down Expand Up @@ -218,7 +221,7 @@ func (c *Cmd) String() string {
// two interfaces with non-comparable underlying types.
func interfaceEqual(a, b interface{}) bool {
defer func() {
recover()
recover() // nolint: errcheck
}()
return a == b
}
Expand Down Expand Up @@ -419,7 +422,7 @@ func (c *Cmd) Start() error {
return err
}

c.Process, err = os.StartProcess(c.Path, c.argv(), &os.ProcAttr{
c.Process, err = osfork.StartProcess(c.Path, c.argv(), &osfork.ProcAttr{
Dir: c.Dir,
Files: c.childFiles,
Env: addCriticalEnv(dedupEnv(envv)),
Expand Down Expand Up @@ -448,7 +451,7 @@ func (c *Cmd) Start() error {
go func() {
select {
case <-c.ctx.Done():
c.Process.Kill()
c.Process.Kill() // nolint: errcheck
case <-c.waitDone:
}
}()
Expand All @@ -459,7 +462,7 @@ func (c *Cmd) Start() error {

// An ExitError reports an unsuccessful exit by a command.
type ExitError struct {
*os.ProcessState
*osfork.ProcessState

// Stderr holds a subset of the standard error output from the
// Cmd.Output method if standard error was not otherwise being
Expand Down
11 changes: 11 additions & 0 deletions internal/stdlib/os/exec/exec_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package exec

import "testing"

// Very rudimentary test that the os/exec fork works.
func TestExec(t *testing.T) {
cmd := Command("ping", "127.0.0.1")
if err := cmd.Run(); err != nil {
t.Fatal(err)
}
}
18 changes: 11 additions & 7 deletions internal/stdlib/os/exec_posix.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,14 @@
package os

import (
"internal/itoa"
"internal/syscall/execenv"
"os"
"runtime"
"syscall"

"github.com/Microsoft/hcsshim/internal/stdlib/execenv"
syscallfork "github.com/Microsoft/hcsshim/internal/stdlib/syscall"

"github.com/Microsoft/hcsshim/internal/stdlib/itoa"
)

// The only signal values guaranteed to be present in the os package on all
Expand All @@ -29,14 +33,14 @@ func startProcess(name string, argv []string, attr *ProcAttr) (p *Process, err e
// UID/GID), double-check existence of the directory we want
// to chdir into. We can make the error clearer this way.
if attr != nil && attr.Sys == nil && attr.Dir != "" {
if _, err := Stat(attr.Dir); err != nil {
pe := err.(*PathError)
if _, err := os.Stat(attr.Dir); err != nil {
pe := err.(*os.PathError)
pe.Op = "chdir"
return nil, pe
}
}

sysattr := &syscall.ProcAttr{
sysattr := &syscallfork.ProcAttr{
Dir: attr.Dir,
Env: attr.Env,
Sys: attr.Sys,
Expand All @@ -52,13 +56,13 @@ func startProcess(name string, argv []string, attr *ProcAttr) (p *Process, err e
sysattr.Files = append(sysattr.Files, f.Fd())
}

pid, h, e := syscall.StartProcess(name, argv, sysattr)
pid, h, e := syscallfork.StartProcess(name, argv, sysattr)

// Make sure we don't run the finalizers of attr.Files.
runtime.KeepAlive(attr)

if e != nil {
return nil, &PathError{Op: "fork/exec", Path: name, Err: e}
return nil, &os.PathError{Op: "fork/exec", Path: name, Err: e}
}

return newProcess(pid, h), nil
Expand Down
95 changes: 10 additions & 85 deletions internal/stdlib/os/exec_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ package os

import (
"errors"
"internal/syscall/windows"
"os"
"runtime"
"sync/atomic"
"syscall"
Expand All @@ -20,19 +20,19 @@ func (p *Process) wait() (ps *ProcessState, err error) {
case syscall.WAIT_OBJECT_0:
break
case syscall.WAIT_FAILED:
return nil, NewSyscallError("WaitForSingleObject", e)
return nil, os.NewSyscallError("WaitForSingleObject", e)
default:
return nil, errors.New("os: unexpected result from WaitForSingleObject")
}
var ec uint32
e = syscall.GetExitCodeProcess(syscall.Handle(handle), &ec)
if e != nil {
return nil, NewSyscallError("GetExitCodeProcess", e)
return nil, os.NewSyscallError("GetExitCodeProcess", e)
}
var u syscall.Rusage
e = syscall.GetProcessTimes(syscall.Handle(handle), &u.CreationTime, &u.ExitTime, &u.KernelTime, &u.UserTime)
if e != nil {
return nil, NewSyscallError("GetProcessTimes", e)
return nil, os.NewSyscallError("GetProcessTimes", e)
}
p.setDone()
// NOTE(brainman): It seems that sometimes process is not dead
Expand All @@ -41,7 +41,7 @@ func (p *Process) wait() (ps *ProcessState, err error) {
// the trick sometimes.
// See https://golang.org/issue/25965 for details.
defer time.Sleep(5 * time.Millisecond)
defer p.Release()
defer p.Release() // nolint: errcheck
return &ProcessState{p.Pid, syscall.WaitStatus{ExitCode: ec}, &u}, nil
}

Expand All @@ -57,12 +57,12 @@ func (p *Process) signal(sig Signal) error {
var terminationHandle syscall.Handle
e := syscall.DuplicateHandle(^syscall.Handle(0), syscall.Handle(handle), ^syscall.Handle(0), &terminationHandle, syscall.PROCESS_TERMINATE, false, 0)
if e != nil {
return NewSyscallError("DuplicateHandle", e)
return os.NewSyscallError("DuplicateHandle", e)
}
runtime.KeepAlive(p)
defer syscall.CloseHandle(terminationHandle)
defer syscall.CloseHandle(terminationHandle) // nolint: errcheck
e = syscall.TerminateProcess(syscall.Handle(terminationHandle), 1)
return NewSyscallError("TerminateProcess", e)
return os.NewSyscallError("TerminateProcess", e)
}
// TODO(rsc): Handle Interrupt too?
return syscall.Errno(syscall.EWINDOWS)
Expand All @@ -75,7 +75,7 @@ func (p *Process) release() error {
}
e := syscall.CloseHandle(syscall.Handle(handle))
if e != nil {
return NewSyscallError("CloseHandle", e)
return os.NewSyscallError("CloseHandle", e)
}
// no need for a finalizer anymore
runtime.SetFinalizer(p, nil)
Expand All @@ -87,86 +87,11 @@ func findProcess(pid int) (p *Process, err error) {
syscall.PROCESS_QUERY_INFORMATION | syscall.SYNCHRONIZE
h, e := syscall.OpenProcess(da, false, uint32(pid))
if e != nil {
return nil, NewSyscallError("OpenProcess", e)
return nil, os.NewSyscallError("OpenProcess", e)
}
return newProcess(pid, uintptr(h)), nil
}

func init() {
cmd := windows.UTF16PtrToString(syscall.GetCommandLine())
if len(cmd) == 0 {
arg0, _ := Executable()
Args = []string{arg0}
} else {
Args = commandLineToArgv(cmd)
}
}

// appendBSBytes appends n '\\' bytes to b and returns the resulting slice.
func appendBSBytes(b []byte, n int) []byte {
for ; n > 0; n-- {
b = append(b, '\\')
}
return b
}

// readNextArg splits command line string cmd into next
// argument and command line remainder.
func readNextArg(cmd string) (arg []byte, rest string) {
var b []byte
var inquote bool
var nslash int
for ; len(cmd) > 0; cmd = cmd[1:] {
c := cmd[0]
switch c {
case ' ', '\t':
if !inquote {
return appendBSBytes(b, nslash), cmd[1:]
}
case '"':
b = appendBSBytes(b, nslash/2)
if nslash%2 == 0 {
// use "Prior to 2008" rule from
// http://daviddeley.com/autohotkey/parameters/parameters.htm
// section 5.2 to deal with double double quotes
if inquote && len(cmd) > 1 && cmd[1] == '"' {
b = append(b, c)
cmd = cmd[1:]
}
inquote = !inquote
} else {
b = append(b, c)
}
nslash = 0
continue
case '\\':
nslash++
continue
}
b = appendBSBytes(b, nslash)
nslash = 0
b = append(b, c)
}
return appendBSBytes(b, nslash), ""
}

// commandLineToArgv splits a command line into individual argument
// strings, following the Windows conventions documented
// at http://daviddeley.com/autohotkey/parameters/parameters.htm#WINARGV
func commandLineToArgv(cmd string) []string {
var args []string
for len(cmd) > 0 {
if cmd[0] == ' ' || cmd[0] == '\t' {
cmd = cmd[1:]
continue
}
var arg []byte
arg, cmd = readNextArg(cmd)
args = append(args, string(arg))
}
return args
}

func ftToDuration(ft *syscall.Filetime) time.Duration {
n := int64(ft.HighDateTime)<<32 + int64(ft.LowDateTime) // in 100-nanosecond intervals
return time.Duration(n*100) * time.Nanosecond
Expand Down
Loading