Hi Thomas,

Pardon the top-posting but ...

The launchMechanism property was introduced by the following issue:

https://bugs.openjdk.java.net/browse/JDK-5049299

at the time there was no CSR process and it went through our internal CCC process. The "specification" was as follows:

---
Specification
jdk.lang.Process.launchMechanism is read once on initialisation. If an invalid value is provided an Error will be thrown immediately stating the reason.

On Linux: jdk.lang.Process.launchMechanism can be set to fork or vfork - defaults to vfork on JDK7 & 8 On Solaris: jdk.lang.Process.launchMechanism can be set to posix_spawn or fork - defaults to fork on JDK7 and posix_spawn on JDK8 On Mac OS X: jdk.lang.Process.launchMechanism can be set to posix_spawn or fork - defaults to posix_spawn on JDK7 & 8
---

so IMHO for this issue the "specification" should simply be:

Update the allowed values of the jdk.lang.Process.launchMechanism property on Linux to accept the value "posix_spawn", which will use the posix_spawn() API. The default value of "vfork" remains unchanged.

---

But as the CSR has already been approved I'm not sure why we are "rearranging the deck chairs".

Cheers,
David
-----



On 5/12/2018 6:05 pm, Thomas Stüfe wrote:
Hi Roger,

thanks for all your help, I appreciate it.

I thought a while and got some fundamental doubts about the whole
process, see inline.

On Fri, Nov 30, 2018 at 9:21 PM Roger Riggs <roger.ri...@oracle.com> wrote:

Hi Thomas,

On 11/30/2018 02:06 PM, Thomas Stüfe wrote:

Hi Roger,

I updated the CSR according to your feedback. I'm a bit at a loss
about the specification though. How should I specify the behavior of
the API without describing the implementation?

What you wrote is fine. It does need to mention posix_spawn by name,
as that is the OS function being used.

The notes about the behavior of libc, fit better as an explanation
in the description of the Solution than in the Specification section.


Also, since this patch only extends an existing implementation to
Linux, I would have thought there are there technical notes in place
describing POSIX_SPAWN on other platforms, which I would have just
refered to. I searched but could not really find anything.

Nope, that's why it was a debugging tool for Martin,
not a documented implementation feature.

See, here I start getting confused. Has this switch ever been an
officially released feature?

If yes, I should be able to find release notes, documentation etc I
could refer to and/or copy from.
If not, why should we file a CSR and a release note for adding an
accepted value to a feature which has never been officially released?

This explains also my problems in formulating the CSR/release note.
How can I describe something without describing its implementation, if
the something is just basically implementation and no feature. It has
no discernible API the user would face, and the consequences for
switching the feature are difficult to describe in 1-2 sentences
without delving deeply into details. 1-2 sentences also run the risk
of giving a false picture.

Basically, I do not see a difference between this switch and any other
diagnostic hotspot switch, e.g., and therefore am undecided on what to
do here.

(Should I redirect these questions to the original mail thread?)


I'm not sure what the proper plural of Unix's is but Unices looks odd.
Perhaps avoid the issue and just say Unix platforms.


Unices is fine I think. See:  https://en.wikipedia.org/wiki/Unix

<quote>
Most common is the conventional Unixes, but _Unices_, treating Unix as
a Latin noun of the third declension, is also popular. The
pseudo-Anglo-Saxon plural form Unixen is not common, although
occasionally seen. Sun Microsystems, developer of the Solaris variant,
has asserted that the term Unix is itself plural, referencing its many
implementations.[42]
</quote>

(Unixen sounds weird and genglish though :)

Thanks, Thomas


On Fri, Nov 30, 2018 at 3:50 PM Roger Riggs <roger.ri...@oracle.com> wrote:

Hi Thomas,

Looks pretty good.

Usually 'we' avoid the first person writing in the jira.
It makes them more readable in the long term, when there is no context for 'we'.

- Describing it as 'experimental' gives the wrong impression
and has some magnified implications as that term is being used for
other major changes.

- The compatibility risk should be corrected:

Supplying an unknown value on the command line produces a java.lang.Error.

% java -Djdk.lang.Process.launchMechanism=POSIX_SPAWN ...

java.lang.Error: POSIX_SPAWN is not a supported process launch mechanism on 
this platform.

- Since CSRs should be self contained, the specification section should 
explicitly
specify the behavior from the API point of view. CSRs should avoid describing
the implementation (though in this case, its not entirely possible).
The webrev of the impl is not relevant.

Thanks, Roger


On 11/30/2018 03:32 AM, Thomas Stüfe wrote:

CSR for jdk12: https://bugs.openjdk.java.net/browse/JDK-8214511

..Thomas



Reply via email to