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

fix: do not clean temporary files of the shell which is running. #284

Merged
merged 1 commit into from
May 23, 2024

Conversation

jan-bar
Copy link
Contributor

@jan-bar jan-bar commented May 21, 2024

fix: #283

A running terminal's temporary directory should never be deleted

Copy link
Member

@aooohan aooohan left a comment

Choose a reason for hiding this comment

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

Looks great. Just the library hasn't been updated in a while.

@jan-bar
Copy link
Contributor Author

jan-bar commented May 22, 2024

Looks great. Just the library hasn't been updated in a while.

After a cursory look at the function of this library, it should be fine, but it is indeed a bit old.

This library is very new, but it has many dependencies and many unused functions.

https://github.com/shirou/gopsutil/blob/89124453c119ce42076b8c0ad84a6c50509776af/process/process.go#L182

@aooohan
Copy link
Member

aooohan commented May 22, 2024

Looks great. Just the library hasn't been updated in a while.

After a cursory look at the function of this library, it should be fine, but it is indeed a bit old.

Actually, we can implement it ourselves, and both libraries are a bit complicated.

For Windows, we can refer the following code:

func (w windowsProcess) Open(pid int) error {
// On Windows, os.FindProcess does not actually find the process.
// So, we use this workaround to get the parent process name.
cmd := exec.Command("tasklist", "/FI", fmt.Sprintf("PID eq %d", pid), "/NH", "/FO", "CSV")
output, err := cmd.Output()
if err != nil {
return err
}
cmd = exec.Command("wmic", "process", "where", fmt.Sprintf("ProcessId=%d", pid), "get", "ExecutablePath", "/format:list")

For Unix-like, we can directly use os.FindProcess function.

@jan-bar
Copy link
Contributor Author

jan-bar commented May 22, 2024

For Unix-like, we can directly use os.FindProcess function.

image

os.FindProcess always returns success under Linux, so it needs to be written as follows

package main

import (
	"flag"
	"os"
	"syscall"
)

func main() {
	pid := flag.Int("p", 0, "")
	flag.Parse()

	err := ProcessIsRun(*pid)
	if err != nil {
		panic(err)
	}
}

func ProcessIsRun(pid int) error {
	proc, err := os.FindProcess(pid)
	if err != nil {
		return err
	}
	defer proc.Release()
	return proc.Signal(syscall.Signal(0))
}

window does not support

image

Processes created by other users cannot be viewed under Linux. If you only consider cleaning up temporary files generated by the current user, there is no problem in doing so.

image

It is impossible to predict whether this scheme will behave correctly on different platforms.

In addition, the above two libraries do not use os.FindProcess when querying process information. There should be a reason.

@aooohan
Copy link
Member

aooohan commented May 23, 2024

Okay. We can merge this now.

@aooohan aooohan changed the title No need to clean the temporary directory of the running terminal fix: do not clean temporary files of the shell which is running. May 23, 2024
@aooohan aooohan merged commit 52ff51c into version-fox:main May 23, 2024
@jan-bar jan-bar deleted the env_cleanup branch May 23, 2024 02:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants