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

Determine if process is running in the foreground #596

Closed
tab1293 opened this issue Oct 19, 2018 · 9 comments
Closed

Determine if process is running in the foreground #596

tab1293 opened this issue Oct 19, 2018 · 9 comments
Assignees

Comments

@tab1293
Copy link

tab1293 commented Oct 19, 2018

Is there any way to determine if a process loaded with process.NewProcess(<pid>) is running in the foreground?

I am looking for similar functionality to what ps -O -p <pid> returns in the STAT column; if there is a + in that column then the process is running in the foreground.

@Lomanic
Copy link
Collaborator

Lomanic commented Oct 20, 2018

This is not possible with gopsutil currently (python psutil doesn't seem to support that either).

If we implement a process.Background() function, we will probably return a ErrNotImplementedError (or a new errors.New("not implemented on Windows")) on Windows which doesn't have this concept at all.

@shirou
Copy link
Owner

shirou commented Oct 23, 2018

gopsutil uses only a first chars on /proc/<pid>/status in linux. So other information like foreground/session leader is discarded.

This is because these status is very different on each platforms, like @Lomanic said. If you make a PR which can abstract these differences, we will merge it with many thanks.

@Lomanic
Copy link
Collaborator

Lomanic commented Oct 24, 2018

From my research:

  • getting foreground status with ps is as simple as checking if the result of ps -o stat= -p $PID has a + (useful for darwin and BSDs which don't have a procfs)
  • GNU procps' ps (and in darwin's adv_cmds in ps/print.c) implements the + state by comparing tpgid and pgrp, which are available on Linux in /proc/$PID/stat at field 5 (pgrp/pgid) and 8 (tpgid ; see procfs' manpage).

I plan to implement this in the coming weeks if nobody does this in the mean time.

@Lomanic Lomanic self-assigned this Oct 25, 2018
@shirou
Copy link
Owner

shirou commented Oct 25, 2018

Thank you for researching @Lomanic .

I think adding a field to Process struct, something like string []substate is better. but if you have any other idea.

And, we can also define const instead of string.

const (
     SubStateSessionLeader = "session leader"
     SubStateForeground = "foreground"
)

@Lomanic
Copy link
Collaborator

Lomanic commented Oct 29, 2018

I already implemented some process.Background functions, are you talking about instead adding a new process.SubState function that would return []string, err?

@shirou
Copy link
Owner

shirou commented Oct 29, 2018

Oh, you already implemented! Great!

My idea was adding a new field to process struct and be return by using process.Status(). If so I think we can not change API just adding and we can handle more state such as process leader. But adding Background() method is good.

@Lomanic
Copy link
Collaborator

Lomanic commented Oct 30, 2018

Indeed, the change you are suggesting (changing process.State() to return []string, err), that I agree with, will be a breaking change for the API (v3 then). I also find it better to use exported constants, for users to compare with, instead of just letters. It would reflect ps STAT column.

@shirou
Copy link
Owner

shirou commented Oct 30, 2018

the change you are suggesting (changing process.State() to return []string, err)

Ah, you are right. forget about my previous comment. Then, process.Background() (bool, err) seems good for me.

I also find it better to use exported constants, ...(snip)

That's a good idea!

@shirou shirou mentioned this issue Oct 30, 2018
9 tasks
@Lomanic
Copy link
Collaborator

Lomanic commented Oct 30, 2018

I added the API change to process.State to the v3 roadmap at #362, I think this should be implemented as it would reflect ps and would simplify the API (instead of having multiple functions, for each sub-state), plus it would read once the result of ps or /proc/$PID/stat(us) for better performance.

Lomanic added a commit to Lomanic/gopsutil that referenced this issue Nov 17, 2018
@Lomanic Lomanic closed this as completed in 878e0a7 Dec 8, 2018
Lomanic added a commit that referenced this issue Dec 8, 2018
Fix #596 Implement process.Background and process.Foreground functions
@shirou shirou mentioned this issue Sep 7, 2020
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants