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

Windows watchParent() fix #42

Merged
merged 6 commits into from
Mar 28, 2020
Merged

Windows watchParent() fix #42

merged 6 commits into from
Mar 28, 2020

Conversation

chicknsoup
Copy link
Contributor

@chicknsoup chicknsoup commented Mar 26, 2020

Slave process watches its parent by sending Signal(0), but as stated in os/exec.go sending Interrupt on Windows is not implemented. This cause prog exits with status code 1 error.

// Signal sends a signal to the Process.
// Sending Interrupt on Windows is not implemented.
func (p *Process) Signal(sig Signal) error {
	return p.signal(sig)
}

This fix uses gopsutil to check for process existence on Windows.

@jpillora
Copy link
Owner

Thanks for the PR!

Looks like this supports linux and windows, but misses the other operating systems. See sys_*.go files for how to do with build comments

@jpillora
Copy link
Owner

Can probably be proc_slave_windows.go // +build windows and proc_slave_others.go // +build !windows (untested)

@chicknsoup
Copy link
Contributor Author

Can probably be proc_slave_windows.go // +build windows and proc_slave_others.go // +build !windows (untested)

Thanks. I forgot other oses as I only build linux and windows binaries.

@jpillora jpillora merged commit 3efe3e1 into jpillora:master Mar 28, 2020
@jpillora
Copy link
Owner

Merged! Thanks :)

@jpillora
Copy link
Owner

Hey @chicknsoup I just added auto testing and the windows build is failing. See https://github.com/jpillora/overseer/actions/runs/65595163

I don't have a windows machine, can you please check this?

@jpillora
Copy link
Owner

I pushed a fix but needs testing

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