Skip to content

fixup! trace2: collect Windows-specific process information #117

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

Merged
merged 1 commit into from
Feb 11, 2019
Merged

fixup! trace2: collect Windows-specific process information #117

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions compat/win32/trace2_win32_process_info.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#include <Psapi.h>
#include <tlHelp32.h>

#define NR_PIDS_LIMIT 42

/*
* Find the process data for the given PID in the given snapshot
* and update the PROCESSENTRY32 data.
Expand All @@ -21,13 +23,17 @@ static int find_pid(DWORD pid, HANDLE hSnapshot, PROCESSENTRY32 *pe32)
}

/*
* Accumulate JSON array:
* Accumulate JSON array of our parent processes:
* [
* exe-name-parent,
* exe-name-grand-parent,
* ...
* ]
*
* We artificially limit this to NR_PIDS_LIMIT to quickly guard against cycles
* in the parent PIDs without a lot of fuss and because we just want some
* context and don't need an absolute answer.
*
* Note: we only report the filename of the process executable; the
* only way to get its full pathname is to use OpenProcess()
* and GetModuleFileNameEx() or QueryfullProcessImageName()
Expand All @@ -38,16 +44,28 @@ static void get_processes(struct json_writer *jw, HANDLE hSnapshot)
{
PROCESSENTRY32 pe32;
DWORD pid;
DWORD pid_list[NR_PIDS_LIMIT];
int k, nr_pids = 0;

pid = GetCurrentProcessId();
while (find_pid(pid, hSnapshot, &pe32)) {
/* Only report parents. Omit self from the JSON output. */
if (nr_pids)
jw_array_string(jw, pe32.szExeFile);

/* We only want parent processes, so skip self. */
if (!find_pid(pid, hSnapshot, &pe32))
return;
pid = pe32.th32ParentProcessID;
/* Check for cycle in snapshot. (Yes, it happened.) */
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to check for cycles or should we just give up once we exceed NR_PIDS_LIMIT?

Copy link
Author

Choose a reason for hiding this comment

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

yea, i could go either way here. my way lets us see "(cycle)" vs "(truncate)" in the Kusto data.
i thought that might be interesting.

Copy link
Member

Choose a reason for hiding this comment

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

If we check for a cycle (which we know happens), we will avoid having NR_PIDS_LIMIT lines of repeated entries every time it does happen...

for (k = 0; k < nr_pids; k++)
if (pid == pid_list[k]) {
jw_array_string(jw, "(cycle)");
return;
}

while (find_pid(pid, hSnapshot, &pe32)) {
jw_array_string(jw, pe32.szExeFile);
if (nr_pids == NR_PIDS_LIMIT) {
Copy link
Member

Choose a reason for hiding this comment

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

Paranoia nit: if (nr_pids >= NR_PIDS_LIMIT) ?

Copy link
Author

Choose a reason for hiding this comment

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

true, but we'd already have a buffer overrun at that point.

jw_array_string(jw, "(truncated)");
return;
}

pid_list[nr_pids++] = pid;

pid = pe32.th32ParentProcessID;
}
Expand Down