[rfc][icedtea-web] spawning linux jvm in rust lunchers

[Alex Kashchenko]

On 10/08/2018 06:15 PM, Jiri Vanek wrote:
>
> [...]
> >>>> 3. please setup std in/out/err handling explicitly (even if you want
>>>> to inherit all of them)
>>>
>>> Well, done, but why?
>>
>> I think standard streams.handling on process spawn are important 
>> enough to be explicit about them.
> 
> Well, from my view, to much early, but sure.
>>
>> The choice to spawn JVM in a separate process (instead of running it 
>> in the same process with Invocation API [1]) looks like a logical 
>> choice to me. But other details about the spawned process 
> I'm not sure with it. In shell scripts we are using exec, but it donot 
> need to persist.
> I would be happy for wider discussion on this topic.
> 
>> are unclear:All questions are rightful, and I do not have much answers.
>>
>> 1. what is an intended lifetime of the launcher process after JVM is 
>> spawned (it currently exits,  > should it linger longer to do 
>> something with the std streams)?
> 
>  From this view, better would be to continue ITW in this same process. 
> Then this decission fade away.

It is not hard to implement in-process launching in rust without 
additional libs (small unsafe block with dlopen and JNI_CreateJavaVM). 
But launching out-of-process allows to easily re-launch ITW from command 
line with exactly the same environment (windows launcher currently logs 
the full java invocation string for that). So I'd rather stay with 
out-of-process spawning.

If needed, launcher process can wait on the child with waitpid [1] and 
do some housekeeping before/after the waiting. As I understand, 
std::process::Command can do exactly that with status() [2].


> Currenly its only purpose can be some stds observation, but it is 
> unlikely rightful.
> It should die, and all lunching logs shoudl be closed. Any other logging 
> should be in java competence.

So current behaviour (exit after spawn) is the one currently intended.


>> 2. end-user clicked JNLP link in Firefox invoking the launcher - what 
>> will happen with standard streams in this case?
> 
> I have seen both - it coudl inherited firefox's streams,  or create its 
> own.
> As I consider terminal as rightfull part of the ITW, they should notbe 
> lost.

I am not sure that we should touch browser stdstreams. Current behaviour 
looks correct for command line launch though.

Going forward, I suggest to handle std streams in "GUI launch" and "CLI 
launch" cases differently - to have some launcher-specific flag ignored 
by ITW to pass the GUI/CLI mode to launcher or something like that.


>> 3. what is the reason to have different handling for stdout and stderr?
> 
> None? Do you see it somewhere?
> Stdout shoudl go to inherited stdout, and stderr to inherited stder.. 
> And stdin form inherrited stdin.

I meant this part "stderr to inherited stder", it may be better to 
redirect stderr to stdout (without requiring user to add 2>&1), but with 
"launch ITW as with bash" reasoning - current logic is fine.


>> Note, that Windows ITW launcher in ojdkbuild drops stdin and redirects 
>> stdout and stderr into a file [2]. This is probably the wrong choice 
>> for cross-platform launcher (you likely need stdin sometimes), but 
>> some conscious decision should be done about std streams.
> 
> Linux one must keep it. And the luncher should be robust enough, to work 
> both with it and without it, and in parallel allowing for both file 
> ends.  So in ideal word, the stdour+err will bee tee-ed-like.

Maybe to have both GUI and CLI "launch modes" supported, but to have CLI 
by default in Linux and GUI by default in Windows.


>
> [...]
> 
> Do you have some idea how to do the exec-like behaviour in rust?

Assuming exec(1) [3] here (not exec(3) [4]), current behaviour (args 
passed though, stdstreams inherited) looks close enough to exec to me. 
Additionally we can also wait for ITW to exit with status() [2] and pass 
its exit code back to shell.


> Attached is updated patch, but I think the spawn x exec-like should be 
> decided now, and should be incorporated.

Patch looks good to me, I guess "await for ITW to exit" and more 
stdstreams handling can be added later.


> 
> [...]
>
[1] https://linux.die.net/man/3/waitpid
[2] https://doc.rust-lang.org/std/process/struct.Command.html#method.status
[3] https://linux.die.net/man/1/exec
[4] https://linux.die.net/man/3/exec


-- 
-Alex