-
Notifications
You must be signed in to change notification settings - Fork 39
GPII-2668: Windows Process Reporter: C# "process not running" exception #156
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
Conversation
Modified C# code to handle the exception and not add the process to the list of running processes.
|
CI job failed: https://ci.gpii.net/job/windows-tests/317/ |
|
ok to test |
|
CI job passed: https://ci.gpii.net/job/windows-tests/318/ |
|
@amb26 Since you so kindly reviewed the original ProcessReporter pull requests, could you have a look at this, when/if you have time? Thanks! |
| } | ||
|
|
||
| public static ProcInfo makeProcInfo(ManagementObject process) { | ||
| public static void makeAndApprendProcInfo(ManagementObject processMO, ArrayList procInfos) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apprend?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hitler apprende
https://www.youtube.com/watch?v=pQD9e9AoV3c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least the error was consistent. Copy/paste is your friend. Or, enemy.
| case ThreadState.Standby: | ||
| case ThreadState.Transition: | ||
| procInfo.state = "Sleeping"; | ||
| break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some tab characters, before "case" on 106-111
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't get these tabs on checkout of this code, but it could be my editor being nice
|
Looks good to me. I couldn't reproduce this naturally, so I changed the code to |
Modified to address Steve Grundell's comments.
|
@stegru wrote:
Yes, I couldn't make the exception happen. The only reason I found out about it was CI, and, even there, it failed only on two occasions. |
|
CI job passed: https://ci.gpii.net/job/windows-tests/319/ |
|
I was able to reproduce the issue quite consistently (every 3rd login or so) as described here: https://issues.gpii.net/browse/GPII-2777 .... It disappeared once I merge with this branch. |
|
@kaspermarkus I had a chat with @stegru in #fluid-tech, and he is satisfied with this code, but he doesn't have the authority to merge it into master: |
Modified the C# ("dotNetProcesses.csx") to handle the exception where the process has exited while fetching information about it, specifically its process ID.