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

[Andrew Azores]

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 spurious wakeups [0] can happen and are probably 
worth handling. In this case I suppose that just 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

-- 
Thanks,

Andrew Azores