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

os: FindProcess always returns process even after that process is killed on Windows (WSL) #33814

Closed
smurfpandey opened this issue Aug 24, 2019 · 16 comments

Comments

@smurfpandey
Copy link

@smurfpandey smurfpandey commented Aug 24, 2019

What version of Go are you using (go version)?

$ go version
go version go1.12.6 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/smurf/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/mnt/f/Pro/Go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/smurf/codebase/f-drive/Go/src/github.com/smurfpandey/what-game/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build264376525=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Given a processId, I am trying to check if a process is still running on Windows 10. The code was built on WSL with GOOS=windows.

I have created a Ticker to check status of a process every 10 second.

func GetProcessStatus(processId int) (string, error) {
	process, err := os.FindProcess(processId)

	if err == nil {
		fmt.Print(true)
		fmt.Printf("Process %d is found", process.Pid)
	} else {
		fmt.Println(false)
	}

	fmt.Println(process)

	return "Watching", nil
}

func JustKeepWatching(processId int) {
	// 1. Start ticker. And just keep watching the process
	tickerForWatcher := time.NewTicker(PROCESS_WATCHER_INTERVAL)
	fmt.Println("Watcher started.")
	go func() {
		for _ = range tickerForWatcher.C {
			_, err := GetProcessStatus(processId)

			if err != nil {
				fmt.Println("Error: ", err)
			} else {
				fmt.Println("still looking", processId)
			}
		}
	}()
}

func main() {
	JustKeepWatching(18864)
	for {
	}
}

I built this code on WSL Linux, ran the output exe on Windows.

What did you expect to see?

I expected that after the watched process is killed, I should see false printed followed by <nil>.

What did you see instead?

Initially the watched process is kept running, and I see
trueProcess 18864 is found&{18864 324 0 {{0 0} 0 0 0 0}}.

After sometime I kill the process 18864, but my watcher application still says true .... When I close the watcher application & start it again, it still says true ...

If I run the powershell command (Get-Process -Id 18864) to check if a process is running, I get proper error message saying no process with that identifier found.

@odeke-em

This comment has been minimized.

Copy link
Member

@odeke-em odeke-em commented Aug 25, 2019

Hello @smurfpandey, thank you for filing this and welcome to the Go project!

On UNIX systems, os.FindProcess is documented to ALWAYS return a non empty Process
https://golang.org/pkg/os/#FindProcess
Screen Shot 2019-08-24 at 7 52 07 PM

Given that your system is Windows Subsystem for Linux (WSL) that might just be that it is the kernel emulating UNIX behavior despite showing Windows.

Investigation

So perhaps to help get to the bottom of this without much noise, I have made for you a program which you can try firstly to run on just a real Windows system to see if that's an issue even on real Windows and then compare with the behavior on WSL.

package main

import (
	"bytes"
	"context"
	"io/ioutil"
	"log"
	"os"
	"os/exec"
	"path/filepath"
	"runtime"
)

var mainSourceCode = []byte(`
package main

import (
	"fmt"
	"log"
	"os"
	"time"
)

func main() {
	defer fmt.Printf("Spent: %s\n", time.Since(time.Now()))

	var b [1]byte
	_, err := os.Stdin.Read(b[:])
	if err != nil {
		log.Fatalf("Failed to read a byte from stdin: %v", err)
	}
}`)

func main() {
	log.SetFlags(0)

	tmpDir, err := ioutil.TempDir("", "killproc")
	if err != nil {
		log.Fatalf("Failed to create temp dir: %v", err)
	}
	defer os.RemoveAll(tmpDir)

	mainFile := filepath.Join(tmpDir, "main.go")
	if err := ioutil.WriteFile(mainFile, mainSourceCode, 0644); err != nil {
		log.Fatalf("Failed to write source code to main file: %v", err)
	}

	// 1. Build the binary from the source code.
	binaryPath := filepath.Join(tmpDir, "killproc")
	output, err := exec.Command("go", "build", "-o", binaryPath, mainFile).CombinedOutput()
	if err != nil {
		log.Fatalf("Failed to build the binary: %v;\nOutput: %s", err, output)
	}

	ctx, cancel := context.WithCancel(context.Background())
	defer cancel()

	// 2. Now that we have the binary, run it.
	binCmd := exec.CommandContext(ctx, binaryPath)
	stdin := new(bytes.Buffer)
	binCmd.Stdin = stdin
	stdoutStderr := new(bytes.Buffer)
	binCmd.Stdout = stdoutStderr
	binCmd.Stderr = stdoutStderr
	if err := binCmd.Start(); err != nil {
		log.Fatalf("Failed to run the binary: %v", err)
	}

	binPid := binCmd.Process.Pid
	// 3. Look for the process in the existent case.
	foundProc, err := os.FindProcess(binPid)
	if err != nil {
		log.Fatalf("Failed to find the process right after starting: %v", err)
	}
	if foundProc == nil {
		log.Fatal("Oddly the found process is nil!")
	}

	// 3.1. Send it that single byte so that it completes.
	stdin.WriteString("Hello, World!")

	// 3.2. Wait for the process to terminate.
	if err := binCmd.Wait(); err != nil {
		log.Fatalf("Failed to end the process: %v", err)
	}

	// Explicitly even cancel the process to ensure it has terminated.
	cancel()

	// By this point we are sure the process has
	// ended and can now examine if it still lingers.
	log.Printf("Output from process: %s\n", stdoutStderr.String())

	// 4. Now search again from the process.
	foundAgainProc, err := os.FindProcess(binPid)
	if err != nil {
		// We are safe! Test passes.
		return
	}

	if foundAgainProc == nil {
		log.Fatal("Oddly err == nil, yet foundAgainProc is nil too")
	}

	switch goos := runtime.GOOS; goos {
	case "windows":
		log.Fatal("Oops issue https://golang.org/issue/33814 still lingers!")

	default:
		// On UNIXes, it is documented as per https://golang.org/pkg/os/#FindProcess
		// that findProcess is a noop.
		log.Printf("On GOOS=%s, os.FindProcess returned a non-blank process, please examine https://golang.org/pkg/os/#FindProcess", goos)
	}
}

If this issue is deemed to be bug, the code I've provided above can be trivially converted into a regression test by replacing uses of log with *testing.T.

With that said, I shall kindly ping some Windows folks @alexbrainman @jordanrh1.

@odeke-em odeke-em changed the title os: os.FindProcess always returns process even after that process is killed on Windows os: FindProcess always returns process even after that process is killed on Windows (WSL) Aug 25, 2019
@smurfpandey

This comment has been minimized.

Copy link
Author

@smurfpandey smurfpandey commented Aug 25, 2019

Hi @odeke-em, just to be clear, I am using WSL only to build the code. But I am running the program on real Windows system.

I will setup Go on WIndows system as well to build & run it their directly.

The whole project is available here: https://github.com/smurfpandey/what-game

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Aug 25, 2019

I expected that after the watched process is killed, I should see false printed followed by <nil>.

Why did you expect that?

Windows is free to reuse PID, once process that uses PID is exited. And Windows can associate new process with the same PID number. So calling os.FindProcess with old PID number will actually find new process.

Alex

@smurfpandey

This comment has been minimized.

Copy link
Author

@smurfpandey smurfpandey commented Aug 25, 2019

I agree, Windows can reuse PID. In my case, I checked and no other process was using that PID. I ran the powershell command (Get-Process -Id 18864) to check if a process with same PID is running, and I get proper error message saying no process with that identifier found.

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Aug 25, 2019

I ran the powershell command (Get-Process -Id 18864) to check if a process with same PID is running, and I get proper error message saying no process with that identifier found.

I am not familiar with powershell. So I cannot comment about that.

On the other hand, os.FindProcess uses Windows OpenProcess API - you can see the source code for yourself. You can also change os.FindProcess code to investigate it yourself - just add lines with prinln. os.FindProcess will only returns true, if OpenProcess succeeds. And I would expect OpenProcess succeed when process exists, and fail when it doesn't.

Alex

@smurfpandey

This comment has been minimized.

Copy link
Author

@smurfpandey smurfpandey commented Aug 25, 2019

@odeke-em I compiled & ran your program on my Windows machine. And this was the output:

Output from process:
Oops issue https://golang.org/issue/33814 still lingers!

Note: I had to make 2 changes in order to run it.

  • Line 49: added .exe in filename. binaryPath := filepath.Join(tmpDir, "killproc.exe")
  • Commented out the step 3.2, that if statement was giving error: Failed to end the process: exit status 1

@alexbrainman I had checked the source code of FindProcess yesterday. I will try to use OpenProcess API directly and see what happens.

@smurfpandey

This comment has been minimized.

Copy link
Author

@smurfpandey smurfpandey commented Aug 25, 2019

I did some digging, and looks like the process gets killed, but it goes into zombie state. This happens when another process has a handle open to it.

In this case, OpenProcess returns a handle, which is never closed. I also verified this by checking the handles my program has. Every 10sec the handle is getting incremented by 1 as the I am calling FindProcess every 10sec.

Fix is to call CloseHandle like it's done here: https://github.com/golang/go/blob/master/src/os/exec_windows.go#L53

@alexbrainman I am not sure how I can change os.FindProcess code to verify this hypothesis.

I can also create a PR for this if you think my hypothesis is correct.

@smurfpandey

This comment has been minimized.

Copy link
Author

@smurfpandey smurfpandey commented Aug 25, 2019

Quick update: I made changes to the FindProcess method locally, but the I am still seeing this issue.
Number handles held by my program didn't go up after the change. But the findprocess is still coming as true.

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Aug 25, 2019

I did some digging, and looks like the process gets killed, but it goes into zombie state. This happens when another process has a handle open to it.

That is normal. That is how Windows works.

In this case, OpenProcess returns a handle, which is never closed.

I don't understand.

Fix is to call CloseHandle

Fix to what?

@alexbrainman I am not sure how I can change os.FindProcess code to verify this hypothesis.

Did you install Go from source? If you did, you can just change os.FindProcess source and rebuild your program, and Go will automatically recompile os package.

I made changes to the FindProcess method locally, but the I am still seeing this issue.

I am still confused. Please, provide, small, but full, program that demonstrates your issue. So we don't guess things.

Alex

@smurfpandey

This comment has been minimized.

Copy link
Author

@smurfpandey smurfpandey commented Aug 25, 2019

Another update: My anti-virus is source problem. It has an open handle to the process which I am watching.

After disabling the anti-virus, and with modified FindProcess, my program is working as expected.

@odeke-em Your program is still not working as expected.

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Aug 25, 2019

My anti-virus is source problem. It has an open handle to the process which I am watching.

I am glad you found the culprit.

Alex

@smurfpandey

This comment has been minimized.

Copy link
Author

@smurfpandey smurfpandey commented Aug 25, 2019

@alexbrainman Sorry for the confusion.

Let me try to give you steps to reproduce this:

  1. Start notepad.exe (or any application). Note down the PID for this application.
  2. Run the below program , passing the PID from above step to JustKeepWatching() in main.
package main

import (
	"fmt"
	"os"
	"time"
)

func GetProcessStatus(processId int) (string, error) {
	

	return "Watching", nil
}

func JustKeepWatching(processId int) {

	// 1. Start ticker. And just keep watching the process
	tickerForWatcher := time.NewTicker(5 * time.Second)
	fmt.Println("Watcher started.")
	go func() {
		for _ = range tickerForWatcher.C {
			process, err := os.FindProcess(processId)
			if err == nil {				
				if process == nil {
					fmt.Println("Process is nil, but err was also nil")		
				} else {
					fmt.Printf("Process %d is found\n", process.Pid)
				}
			} else {
				fmt.Println("Process not found", err)
			}	
		}
	}()

}

func main() {
	JustKeepWatching(12856)
	for {
	}
}
  1. You should see Process XXXX is found printed to console every 5 seconds. XXXX is PID from 1st step.
  2. Close the notepad application started in step 1.

Actual Result:
The program keeps on printing Process XXXX is found.

Expected Result
The program should print Process not found after the notepad application is closed.

This is happening because the process being watched goes into zombie state. Reason for zombie state is handle not closed in os.FindProcess. In FindProcess we are calling OpenProcess API of Windows, which returns a handle. This handle should be closed & released. And the fix for this issue is to call defer syscall.CloseHandle(h) just before the return statement. After making the change, the above program works as expected.

Original Source:

func findProcess(pid int) (p *Process, err error) {
	const da = syscall.STANDARD_RIGHTS_READ |
		syscall.PROCESS_QUERY_INFORMATION | syscall.SYNCHRONIZE
	h, e := syscall.OpenProcess(da, false, uint32(pid))
	if e != nil {
		return nil, NewSyscallError("OpenProcess", e)
	}
	return newProcess(pid, uintptr(h)), nil
}

For e.g In os.terminateProcess(), the CloseHandle method is being called correctly.

smurfpandey added a commit to smurfpandey/go that referenced this issue Aug 25, 2019
Close the handle to avoid process becoming zombie.
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Aug 25, 2019

Change https://golang.org/cl/191817 mentions this issue: os: close the handle returned by OpenProcess

@smurfpandey

This comment has been minimized.

Copy link
Author

@smurfpandey smurfpandey commented Aug 25, 2019

So on further thinking, I guess os.FindProcess is working as expected. But the way I am using where I am calling FindProcess again & again, number of open handle keeps on increasing, so in my case it's not working.

I just want to know if process is still alive, and I don't want to keep the handle open.

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Aug 26, 2019

But the way I am using where I am calling FindProcess again & again, number of open handle keeps on increasing, so in my case it's not working.

I think you want to call

https://golang.org/pkg/os/#Process.Release

after you are done with the process returned by os.FindProcess.

Alex

@smurfpandey

This comment has been minimized.

Copy link
Author

@smurfpandey smurfpandey commented Aug 26, 2019

I decided to not use FindProcess, since even if my application doesn't hold any handle, other applications(like anti-virus) can keep a handle making the process to go into Zombie state. And I will still be in same situation.

I decided to use another library (go-ps) to check if process exists or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants
You can’t perform that action at this time.