Re: RFR 9: 8034903: Add Logging of Process.start arguments and resulting pid
On 15/02/2014 18:27, Martin Buchholz wrote: : OK, so is this code for the benefit of real users or openjdk developers? If the latter, it doesn't pull its weight - Dude, you forgot to remove your printf debugging statements. If the former, this feature needs to be explained somewhere - your JDK will randomly start doing more logging that users didn't ask for and is undocumented. I think Roger needs to expands on the motivation a bit but if logging is being added here then I think we need to understand how it might fit into a possible larger effort to improve the overall supportability. -Alan.
Re: RFR 9: 8034903: Add Logging of Process.start arguments and resulting pid
On 02/13/2014 10:26 PM, roger riggs wrote: Having folks stumbling over process creation and problems of quoting, especially on windows, it seems useful to log the native commands and arguments. They are proposed to be logged using the PlatformLogger at Level.FINE which will not be logged by default. The environment is useful in some cases, but verbose, that it should be Logged at FINER. The pid of the spawned process logged as well for traceability. It may make sense to expose something like getProcessID() to the public because it is independently useful on systems with /proc. In the src/solaris version, Arrays::toString(Object[]) does not quote the arguments, so the argument boundary is lost in the logging. I think something that performs quoting (using the Java language rules, not shell) would be helpful in this context. Although on non-Windows platforms, this information can easily be obtained with tools such as strace or truss. -- Florian Weimer / Red Hat Product Security Team
Re: RFR 9: 8034903: Add Logging of Process.start arguments and resulting pid
On 14/02/2014 10:26, Florian Weimer wrote: It may make sense to expose something like getProcessID() to the public because it is independently useful on systems with /proc. Yes, this has been asked for many times. We have JEP 102 [1] that lists this and other Process topics that we'd like to get funded for JDK 9. -Alan [1] http://openjdk.java.net/jeps/102
Re: RFR 9: 8034903: Add Logging of Process.start arguments and resulting pid
Hi Roger,
I agree with Paul that using Objects.toString(environment) would be
cleaner.
Concerning the test, you may want to modify CheckHandler.publish to
filter out any log record that doesn't come from your ProcessImpl
classes. Something like:
175 @Override
176 public void publish(LogRecord lr) {
// Only look at the records emitted by the
// java.lang.Process logger
if (java.lang.Process.equals(lr.getLoggerName()) {
177logRec = lr;
}
178 }
Also, since you're changing the Level of the root logger, you may
want to either run your test in /othervm mode, or make sure that it
puts back the root logger level to its original value and also
removes the CheckHandler() instance from the root handlers with a try {
} finally { } to avoid polluting the other tests that will follow.
best regards,
-- daniel
On 2/13/14 10:26 PM, roger riggs wrote:
Hi,
Having folks stumbling over process creation and problems of quoting,
especially on windows, it seems useful to log the native commands and
arguments.
They are proposed to be logged using the PlatformLogger at Level.FINE
which will not be logged by default. The environment is useful in some
cases,
but verbose, that it should be Logged at FINER. The pid of the spawned
process
logged as well for traceability.
Please take a look at this first draft and comment on whether it is useful,
a good idea and any improvements needed.
Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-log-processcreate/
Thanks, Roger
Re: RFR 9: 8034903: Add Logging of Process.start arguments and resulting pid
Thanks Paul, Daniel,
Thanks for the suggestions, I updated the webrev.
/othervm does seem to be needed to prevent this test from interfering or
being interfered wit.
Roger
On 2/14/2014 6:06 AM, Daniel Fuchs wrote:
Hi Roger,
I agree with Paul that using Objects.toString(environment) would be
cleaner.
Concerning the test, you may want to modify CheckHandler.publish to
filter out any log record that doesn't come from your ProcessImpl
classes. Something like:
175 @Override
176 public void publish(LogRecord lr) {
// Only look at the records emitted by the
// java.lang.Process logger
if (java.lang.Process.equals(lr.getLoggerName()) {
177logRec = lr;
}
178 }
Also, since you're changing the Level of the root logger, you may
want to either run your test in /othervm mode, or make sure that it
puts back the root logger level to its original value and also
removes the CheckHandler() instance from the root handlers with a try {
} finally { } to avoid polluting the other tests that will follow.
best regards,
-- daniel
On 2/13/14 10:26 PM, roger riggs wrote:
Hi,
Having folks stumbling over process creation and problems of quoting,
especially on windows, it seems useful to log the native commands and
arguments.
They are proposed to be logged using the PlatformLogger at Level.FINE
which will not be logged by default. The environment is useful in some
cases,
but verbose, that it should be Logged at FINER. The pid of the spawned
process
logged as well for traceability.
Please take a look at this first draft and comment on whether it is
useful,
a good idea and any improvements needed.
Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-log-processcreate/
Thanks, Roger
Re: RFR 9: 8034903: Add Logging of Process.start arguments and resulting pid
Hi Florian, As for escaping, is there an existing API for encoding and decoding those? A human won't have trouble figuring out the few cases where there might be some ambiguity due to , in the string. The test will be more complicated as well and I think added that the extra complexity is not a good investment. Thanks, Roger On 2/14/2014 5:26 AM, Florian Weimer wrote: On 02/13/2014 10:26 PM, roger riggs wrote: Having folks stumbling over process creation and problems of quoting, especially on windows, it seems useful to log the native commands and arguments. They are proposed to be logged using the PlatformLogger at Level.FINE which will not be logged by default. The environment is useful in some cases, but verbose, that it should be Logged at FINER. The pid of the spawned process logged as well for traceability. It may make sense to expose something like getProcessID() to the public because it is independently useful on systems with /proc. In the src/solaris version, Arrays::toString(Object[]) does not quote the arguments, so the argument boundary is lost in the logging. I think something that performs quoting (using the Java language rules, not shell) would be helpful in this context. Although on non-Windows platforms, this information can easily be obtained with tools such as strace or truss.
Re: RFR 9: 8034903: Add Logging of Process.start arguments and resulting pid
On 14/02/2014 18:10, Martin Buchholz wrote: I'm not excited about having logging statements added throughout the jdk implementation. PlatformLogger is an implementation detail, so real users cannot benefit. Your OS should already provide facilities to let you trace process creation. E.g. on Linux you can do this with strace (and I have done this myself in the past to debug subprocess creation). So I'd say adding logging just to provide a small rarely used convenience for jdk developers is Not Worthwhile. Just on PlatformLogger then it just forwards to j.u.logging when it is present. -Alan.
RFR 9: 8034903: Add Logging of Process.start arguments and resulting pid
Hi, Having folks stumbling over process creation and problems of quoting, especially on windows, it seems useful to log the native commands and arguments. They are proposed to be logged using the PlatformLogger at Level.FINE which will not be logged by default. The environment is useful in some cases, but verbose, that it should be Logged at FINER. The pid of the spawned process logged as well for traceability. Please take a look at this first draft and comment on whether it is useful, a good idea and any improvements needed. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-log-processcreate/ Thanks, Roger
Re: RFR 9: 8034903: Add Logging of Process.start arguments and resulting pid
Roger, I only have two suggested improvements: 1) solaris/../ProcessImpl.java should use Objects.toString() rather than the tenary operator for a string choice. You did this alright in /windows/../ProcessImpl.java 2) /windows/../ProcessImpl.java doesn't need to specify null for Objects.toString() since that is its default output for null input. Cheers, Paul On Thu, Feb 13, 2014 at 3:26 PM, roger riggs roger.ri...@oracle.com wrote: Hi, Having folks stumbling over process creation and problems of quoting, especially on windows, it seems useful to log the native commands and arguments. They are proposed to be logged using the PlatformLogger at Level.FINE which will not be logged by default. The environment is useful in some cases, but verbose, that it should be Logged at FINER. The pid of the spawned process logged as well for traceability. Please take a look at this first draft and comment on whether it is useful, a good idea and any improvements needed. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-log-processcreate/ Thanks, Roger -- Cheers, Paul
