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

Usage of getprocs on AIX possibly wrong #60

Open
djboris9 opened this issue Nov 7, 2022 · 1 comment · May be fixed by #61
Open

Usage of getprocs on AIX possibly wrong #60

djboris9 opened this issue Nov 7, 2022 · 1 comment · May be fixed by #61

Comments

@djboris9
Copy link
Contributor

djboris9 commented Nov 7, 2022

Hi

getprocs uses an IndexPointer under AIX, which doesn't need to correlate in every case to the PID.

The usage is correct in FetchPids, where we start with 0 and iterate through the process table:

pid := C.pid_t(0)
procMap := make(ProcsMap, 0)
var plist []ProcState
for {
// getprocs first argument is a void*
num, err := C.getprocs(unsafe.Pointer(&info), C.sizeof_struct_procsinfo64, nil, 0, &pid, 1)

But in GetInfoForPid we are using the IndexPointer as the pid:

func GetInfoForPid(_ resolve.Resolver, pid int) (ProcState, error) {
info := C.struct_procsinfo64{}
cpid := C.pid_t(pid)
num, err := C.getprocs(unsafe.Pointer(&info), C.sizeof_struct_procsinfo64, nil, 0, &cpid, 1)

And in FillPidMetrics the same:

func FillPidMetrics(_ resolve.Resolver, pid int, state ProcState, filter func(string) bool) (ProcState, error) {
pagesize := uint64(os.Getpagesize())
info := C.struct_procsinfo64{}
cpid := C.pid_t(pid)
num, err := C.getprocs(unsafe.Pointer(&info), C.sizeof_struct_procsinfo64, nil, 0, &cpid, 1)

Now the problem occurs. Since we have the count parameter set to 1 in GetInfoForPid and FillPidMetrics, we have a matching IndexPointer==pi_pid at least in all of my tests with:

#include <procinfo.h>
#include <sys/types.h>

#define MAXPROCS 1
 
int main() {
        int index = 0;
        struct procsinfo64 pinfo[MAXPROCS];

        for (;;) {
                int curindex = index;
                int numproc = getprocs(pinfo, sizeof(struct procsinfo64), NULL, 0, &index, MAXPROCS);
                if (numproc < 1) {
                        break;
                }

                int i;
                for (i = 0;i < numproc; i++) {
                        printf("%-6d->%-6d %-16s\n", curindex, pinfo[i].pi_pid, pinfo[i].pi_comm);
                }
        }

        return 0;
}
/*
Output:
0     ->0                      
1     ->1      init            
260   ->260    wait            
65798 ->65798  sched           
131336->131336 lrud            
196874->196874 vmptacrt        
262412->262412 psmd            
327950->327950 vmmd            
393488->393488 pvlist          
459026->459026 reaffin         
524564->524564 memgrdd         
590112->590112 lvmbb
...
*/

Now I'm not sure if IndexPointer==pi_pid can be guaranteed for count=1 and in my opinion, we should assert the requested pid matches the info.pi_pid in the CGo functions.
But also if the process/pid was stopped in the mean time, usually the resulting ProcessBuffer has info.pi_state=Running set, which is wrong.

Therefore: Do we need to ensure that the PID exists before calling of especially GetInfoForPid or should the GetInfoForPid be modified to check first if the process exists by iterating through the getprocs index pointer? I think the second option behaves more similar to the Linux version.

I'm asking this because I'm implementing AIX support for process.GetPIDState.

@djboris9
Copy link
Contributor Author

djboris9 commented Nov 7, 2022

An example process existence check is prepared here: acba645

func procExists(pid int) (bool, error) {
	info := [20]C.struct_procsinfo64{}
	idxPtr := C.pid_t(0)

	for {
		num, err := C.getprocs(unsafe.Pointer(&info), C.sizeof_struct_procsinfo64, nil, 0, &idxPtr, 20)
		if err != nil {
			return false, fmt.Errorf("error fetching PID at IndexPointer %d: %w", int(idxPtr), err)
		}

		if num == 0 {
			break
		}

		for i := 0; i < len(info); i++ {
			if int(info[i].pi_pid) == pid {
				return true, nil
			}
		}

	}

	return false, nil
}

@djboris9 djboris9 linked a pull request Nov 8, 2022 that will close this issue
4 tasks
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 a pull request may close this issue.

1 participant