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

[Jiri Vanek]

On 10/4/18 11:21 PM, Alex Kashchenko wrote:
> On 10/04/2018 05:03 PM, Jiri Vanek wrote:
>> Thank y ou for review!
>>
>> [...]
>>
>>>>      * rust-launcher/src/os_access.rs: (Os) declared new method ofspawn_java_process. (Linux) 
>>>> implementing it by spawn
>>>
>>> 1. Don't see a reason to use copy/move for its arguments, I'd rather use:
>>>
>>> jre_dir: &std::path::PathBuf, args: &Vec<String>
>>
>> Uf, That would mean, that javadir going in,  will change to path to java bin. That is unexpected. 
>> So I kept original
> 
> M, this reasoning is not clear, currently you are copying the path on the function enter and then 
> immediately copying it second time with the clone. This doesn't make sense to me. Not that it is any 
> real difference in this case, still there should be some strong reason to NOT pass string-like 
> object by const reference.

My bad. Using reference now and cloning justifiably. >
> 
>>
>> [...]
>>
>>> 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.
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.
> 
> 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.
> 
> 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.
> 
> 
> 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.

> 
> 
>>>
>>> 4. in error message a space is needed between "," and "\"
>>
>> nice catch
> 
> This is not actually included in the updated patch (very minor thing of course).
> 

arghhh... sorry... fixed.
>>
>> [...]

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

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

>>
> 
> [1] https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/invocation.html
> [2] 
> https://github.com/ojdkbuild/contrib_itw-launcher/blob/75dea38ab870083978675059b7c11d0994172ba9/src/launcher.cpp#L379 
> 
> 
>>
>> [...]
>>
> 
Thanx a lot!
   J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: spawning3.patch
Type: text/x-patch
Size: 2307 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20181008/b07b651b/spawning3.patch>