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

[Andrew Azores]

On 15/06/15 01:02 PM, Jiri Vanek wrote:
> On 06/15/2015 06:58 PM, Andrew Azores wrote:
>> 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.
>>
> Fair enough.
>
>
> There is one one related waitFor in Xdesktopentry. and few more over ITW
>
> I'm for encapsualting
>
> >>>  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);
> >>>                  }
> >>>              }
>
> I'm for separate it into  method waitForSafely(Process p), and use in 
> all palces across ITW (where the waitFor is used, and the 
> InterruptedException is cacth and just logged.
>
>
> oook?
>
>
> Thanx!
>
>  J.

+1

-- 
Thanks,

Andrew Azores