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.

Sign up for GitHub

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

Jump to bottom

Fix checkpid on windows #2031

Closed
wants to merge 4 commits into from
Closed

Fix checkpid on windows #2031

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fbd2c09
Syscall.signal(0) always returns false on windows
hurricanehrndz May 22, 2024
41b2f6e
Update ui pid file name
hurricanehrndz May 22, 2024
d99f867
Update client/ui/client_ui.go
hurricanehrndz May 23, 2024
c4b4566
Update client/ui/client_ui.go
hurricanehrndz May 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 33 additions & 6 deletions client/ui/client_ui.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package main

import (
"bufio"
"context"
_ "embed"
"flag"
Expand Down Expand Up @@ -473,7 +474,6 @@ func (s *serviceClient) updateStatus() error {
Stop: backoff.Stop,
Clock: backoff.SystemClock,
})

if err != nil {
return err
}
Expand Down Expand Up @@ -716,16 +716,43 @@ func openURL(url string) error {

// checkPIDFile exists and return error, or write new.
func checkPIDFile() error {
pidFile := path.Join(os.TempDir(), "wiretrustee-ui.pid")
pidFile := path.Join(os.TempDir(), "netbrid-ui.pid")
hurricanehrndz marked this conversation as resolved.
Show resolved Hide resolved
if piddata, err := os.ReadFile(pidFile); err == nil {
if pid, err := strconv.Atoi(string(piddata)); err == nil {
if process, err := os.FindProcess(pid); err == nil {
if err := process.Signal(syscall.Signal(0)); err == nil {
return fmt.Errorf("process already exists: %d", pid)
}
if pidExists(pid) {
return fmt.Errorf("process already exists: %d", pid)
}
}
}

return os.WriteFile(pidFile, []byte(fmt.Sprintf("%d", os.Getpid())), 0o664) //nolint:gosec
}

func pidExists(pid int) bool {
if runtime.GOOS != "windows" {
if process, err := os.FindProcess(pid); err == nil {
if err := process.Signal(syscall.Signal(0)); err == nil {
return true
}
}
}
// Prepare the command
cmd := exec.Command("tasklist", "/FI", fmt.Sprintf("PID eq %d", pid))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you use a function similar to GetSystem32Command to avoid path issues?


// Get the output
output, err := cmd.Output()
if err != nil {
return false
hurricanehrndz marked this conversation as resolved.
Show resolved Hide resolved
}

// Parse the output
scanner := bufio.NewScanner(strings.NewReader(string(output)))
for scanner.Scan() {
line := scanner.Text()
if strings.Contains(line, fmt.Sprintf("%d", pid)) {
return true
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you handle scanner.Err() here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If scanner.Err actually returns an error the overall return of the function would be false, the only benefit to handling scanner.Err is to log an issue with the scanning, but it is not like the log is written to disk or anything so ...

Copy link
Contributor

@lixmal lixmal May 23, 2024 •
edited

Choose a reason for hiding this comment

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

Right, but if there's an issue that would at least give us a hint when debugging the application from CLI


return false
}