Improve Timeout and Error Handling #22
Conversation
Added methods for executing script from a BufferedReader. This is neccessary if you want to execute scripts that are bundled in a jar File (http://stackoverflow.com/a/8258308/3601364) +executeScript(BufferedReader srcReader) +executeScript(BufferedReader srcReader, String params)
When starting a new ProcessBuilder process it does not load the defaultCharset (example: windows-1252), instead it loads some other charset. (example: ibm850). Read more about this issue here: http://stackoverflow.com/questions/22349139/utf-8-output-from-powershell/22350989 In the jPowerShell Code: Java tries to read the Stream with the defaultCharset or UTF-8 but both wont work due to the mismatch! Instead, we just need to set the Encoding that ProcessBuilder/Powershell uses to the defaultCharset. See the code of this commit how it can be done.
File for Codepage lookup generated from this source: https://msdn.microsoft.com/de-de/library/windows/desktop/dd317756(v=vs.85).aspx
Whops.. forgot that while editing directly in the Webbrowser..
Add description for new Methods
Corrected Typo..
For timed out commands the stdin is blocked, the close of the child process must be forced. Note: Process has no ".getPid()" (available only in Java 9). Therefore using JNA to determine the PID of the child process where powershell is running.
If you want to avoid including this library, an alternative way to determine the child PID of the cmd process that hosts powershell.exe would be:
Get the PID of the JVM:
ManagementFactory.getRuntimeMXBean().getName().split("@")[0]
Use this value to Runtime.exec(wmic process where (ParentProcessId=1234) get ProcessId)
Parse the PID from the response.
(Travis CI build) not available in 1.5
|
Hi Harinus, Thanks for your MR. I have just briefly analyse it. Il like the idea of handle the timeout case and try to kill the process, however I do not like the JNA approach:
Regarding the timestamp, I do not see the point of generate it in PowerShell component and pass it by the contructor, etc. Don't you think it could work also if we declare it as local variable on PowerShellCommandProcessor.startReading method? Thanks for your help! |
|
Greetings Professor Falken, Java 9 would solve the dependency issue. A function Process.getPid() exists there.. If you find any cross platform solution to determine PID of a Process that was started within a JVM let me know. An alternative would be to throw an exception instead of killing the process. However this should be handled in some way, as it can cause OutOfMemory / crashes when misused excessively. The timestamp was generated in the PowerShell component on purpose. |
2 participants
The Timeout handling was previously a little bit unreliable and scripts could possibly run longer than maxWait. Calculating the timeout based on a timestamp now.
Also, checking several constellations when the script finishes and handle then appropriately:
-1)Command finishes as expected:
-2)Command finishes with errors (no output)
-3)Command finished with errors (output)
-4)Command times out (no output)
-5)Command times out (output)
-6)Command finished, but jPowerShell fails for any reason
A timed out command results in a blocked process if it already has some output. Added a snippet that hard terminates this process to avoid dangling resources.
Note: Check the commit messages, I added some information on, how the usage of JNA can be avoided if neccesary.
Note2: Please cherrypick the latest 4 commits, do not know why the others are there nor how to remove them.. sorry for the mess.
Note3: Might also fix #21 #13 #11 -> Needs some testing, I did not encounter any problems yet.
You might add test-cases using the following scripts:
1)
Write-Host hi!
2)
Get-Unique -help
3)
Write-Host hi!
Wriste-Host Hallo!
4)
Start-Sleep -s 60
5)
Write-Host hi!
Start-Sleep -s 60