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

[Andrew Azores]

On 15/06/15 12:52 PM, Jiri Vanek wrote:
> On 06/15/2015 06:37 PM, Andrew Azores wrote:
>> On 15/06/15 12:26 PM, Jiri Vanek wrote:
>>>
>>>> I don't know about "keep going" and what that would actually mean 
>>>> in this case and what kind of
>>>> behaviour would arise. "Die" sounds reasonable, though, but then we 
>>>> need to check if p is actually
>>>> finished after we wake up and then decide to die if it isn't - so 
>>>> then why not instead of dying just
>>>> waitFor p again?
>>>
>>>
>>> May you provide code snippet please?
>>
>>              ProcessBuilder pb = new ProcessBuilder(command);
>>              pb.inheritIO();
>>              Process p = pb.start();
>>              boolean pTerminated = false;
>>              while (!pTerminated) {
>>                  try {
>>                      p.waitFor();
>>                  } catch (InterruptedException e) {
>> OutputController.getLogger().log(OutputController.Level.ERROR_ALL, e);
>>                  }
>>                  try {
>>                      p.exitValue();
>>                      pTerminated = true;
>>                  } catch (IllegalThreadStateException e) {
>> OutputController.getLogger().log(OutputController.Level.ERROR_ALL, e);
>>                  }
>>              }
>>
>> Hopefully the formatting survives.
>
> Does :) Thanx!
>>
>>>
>>> Is this kind of awaking really worthy of surviving?  I'm really 
>>> inclined to just catch, log, and
>>> continue ITW running...
>>>
>>> J.
>>
>> It's not that hard to correctly handle it, although the code does 
>> look pretty ugly. I think the
>> better question to ask is if a spurious wakeup does occur here, will 
>> ITW continue to run properly?
>> Because like I said, it's possible that the wakeup occurs and there 
>> is no exception to catch or log,
>> and so you just end up with a silent failure later on.
>
> I really don't know. I'm still a bit more inclined to my straight 
> original version.
> If you insists on this one you posted, please state it (jsut for record).
>
> I'm probably unable to provide any more decision points:(
>
> J.

I "gently insist". I think mine is a more correct approach to take, but 
it is targeting what should be a very rare edge case and so in practice 
your cleaner code should work just as well. I just think it's worthwhile 
to add the slightly dirty loop and exitValue stuff to make sure that 
this code hunk never makes us have to worry about what happens if the 
calling Launcher thread ends up running in parallel with the forked 
process, because that doesn't sound easy to analyze and does sound like 
it would not lead to correct results - and above all, would be extremely 
hard to track and reproduce.

-- 
Thanks,

Andrew Azores