[rfc][icedtea-web] replace runtime().exec by processbuilder

[Jiri Vanek]

On 06/15/2015 06:11 PM, Andrew Azores wrote:
> On 15/06/15 11:57 AM, Jiri Vanek wrote:
>> On 06/15/2015 05:46 PM, Andrew Azores wrote:
>>> On 15/06/15 11:34 AM, Jiri Vanek wrote:
>>>> ssia
>>>>
>>>>
>>>> I'm also in temptation to push this to 1.6
>>>>
>>>> The handling of streams is just terrible :(
>>>>
>>>>
>>>> J.
>>>
>>>> -            Process p = Runtime.getRuntime().exec(command);
>>>> -            new StreamEater(p.getErrorStream()).start();
>>>> -            new StreamEater(p.getInputStream()).start();
>>>> -            p.getOutputStream().close();
>>>> -
>>>> +            ProcessBuilder pb = new ProcessBuilder(command);
>>>> +            pb.inheritIO();
>>>> +            Process p =pb.start();
>>>> +            try {
>>>> +                p.waitFor();
>>>> +            } catch (InterruptedException e) {
>>>> + OutputController.getLogger().log(OutputController.Level.ERROR_ALL, e);
>>>> +            }
>>>
>>> Where does the "waitFor" come from? I'm not seeing that the old code does the same thing (but maybe
>>
>> Surprisingly no - the change of behavior   is to not include it.
>> The wait is hidden in the handling of streams.
>> As streaminheritance is different, then the waitFor is the necessarything to do.
>>
>
> Ah alright. I figured that might have been what was happening but it wasn't clear even when reading
> the StreamEater that it sort of implicitly caused a waitFor effect. This way is much, much better then!
>
>>
>>> I'm just missing it and it does). Is this meant to be a change in behaviour? If that
>>> InterruptedException does get thrown, is it ok that the current thread continues on in parallel with
>>> process p?
>>
>> For that I dont have answer. Whats your suggestion? HAve you ever seenthis exception? :)
>>
>> J
>>
>
> Well, only when I've been intentionally trying to interrupt and wake up a waiting thread. But

yes:)
> spurious wakeups [0] can happen and are probably worth handling. In this case I suppose that just
hmmm

> means putting the waitFor into a loop and continuing to waitFor until p.exitStatus() returns rather
> than throws IllegalThreadStateException? Sounds pretty messy :(
>
> [0] https://en.wikipedia.org/wiki/Spurious_wakeup


             Process p =pb.start();
boolean workaround = true;
do{
             try {
                p.waitFor();
woraround=false;
            } catch (InterruptedException e) {
	 OutputController.getLogger().log(OutputController.Level.ERROR_ALL, e);
	  }
}while(workaround);

??

ISnt better jsut keep going or die?