Hi!
>> > If the limit is the same for all platforms, then the fix
>> > could be improved to check that each line is smaller than the
>> > limit before to send it to the target JVM, and properly
>> > report an error if it isn't. Right now, each platform
>> > throws an IOException with a platform dependent message.
>> > Instead, detecting lines exceeding the limit and printing
>> > a clear message like:
>> >
>> > "line 84: Line too long"
>> >
>> > would improve the user experience.
>>
>> That should be doable. Are we OK with hard-coding the current VM limit
>> into the client side code?
>
> I'm not fond of the idea that the client side "knows" the VM limit.
> Yes, I recognize that would improve the user experience, but it
> makes the code more fragile. I don't think there is a client-side
> query for determining the buffer length. Perhaps you should query
> Alan Bateman for advice since the attach-on-demand stuff was his
> creating way back when... He might have some advice...
I talked with Dan over IM about this, and feel that it is not worth
pursuing. There is no obvious interface for a client to dynamically
query the limit. And when an exception is thrown, there is no obvious
way for the client to distinguish between having sent a string that was
too large and some other IO error. Hard coding this value into the
client is just asking for trouble. Ideally, jcmd should be
inter-operable with multiple versions of the JVM.
Thanks for all the feedback. Will rebuild / re-test and get a new
version out ASAP.
Cheers,
-Buck
On 03/29/12 23:29, Daniel D. Daugherty wrote:
Resending... with David B on the reply...
On 3/29/12 5:49 AM, David Buck wrote:
Hi Frederic,
Thank you for the review!
> I have a question about the 1024 bytes limit.
> Have you tested this limit on the different platforms?
I tested on Solaris, Linux and Windows. They were all 1024.
> Because jcmd uses the attachAPI to send commands to
> the target JVM and each platform uses a different mechanism
> to implement the attachAPI (Solaris->door, Linux->socket,
> Windows->Pipe). I can see the 1024 bytes limit on the
> Windows implementation, but I don't see hard coded values
> on other platforms. Do you know what is the limit for
> each platform?
The 1024 byte limit is hard-coded into hotspot. See arg_length_max in
hotspot/src/share/vm/services/attachListener.hpp.
> If the limit is the same for all platforms, then the fix
> could be improved to check that each line is smaller than the
> limit before to send it to the target JVM, and properly
> report an error if it isn't. Right now, each platform
> throws an IOException with a platform dependent message.
> Instead, detecting lines exceeding the limit and printing
> a clear message like:
>
> "line 84: Line too long"
>
> would improve the user experience.
That should be doable. Are we OK with hard-coding the current VM limit
into the client side code?
I'm not fond of the idea that the client side "knows" the VM limit.
Yes, I recognize that would improve the user experience, but it
makes the code more fragile. I don't think there is a client-side
query for determining the buffer length. Perhaps you should query
Alan Bateman for advice since the attach-on-demand stuff was his
creating way back when... He might have some advice...
> src/share/classes/sun/tools/jcmd/JCmd.java
> Copyright years should be "2011, 2012" instead of "2012"
Sorry, not sure what I was thinking. Fixed.
> test/sun/tools/jcmd/dcmd-big-script.txt
> The empty line at the end of the line might cause the
> VM to complain about an unknown diagnostic command
After the fix, blank lines no longer throw that error, but still, the
blank line was unintentional. I removed it.
Hmmm... But what should happen when a blank line is sent and do we
have a test for that... :-)
Dan
Cheers,
-Buck
On 03/29/12 19:34, Frederic Parain wrote:
Hi David,
I have a question about the 1024 bytes limit.
Have you tested this limit on the different platforms?
Because jcmd uses the attachAPI to send commands to
the target JVM and each platform uses a different mechanism
to implement the attachAPI (Solaris->door, Linux->socket,
Windows->Pipe). I can see the 1024 bytes limit on the
Windows implementation, but I don't see hard coded values
on other platforms. Do you know what is the limit for
each platform?
If the limit is the same for all platforms, then the fix
could be improved to check that each line is smaller than the
limit before to send it to the target JVM, and properly
report an error if it isn't. Right now, each platform
throws an IOException with a platform dependent message.
Instead, detecting lines exceeding the limit and printing
a clear message like:
"line 84: Line too long"
would improve the user experience.
A few comments on the code:
src/share/classes/sun/tools/jcmd/JCmd.java
Copyright years should be "2011, 2012" instead of "2012"
test/sun/tools/jcmd/dcmd-big-script.txt
The empty line at the end of the line might cause the
VM to complain about an unknown diagnostic command
test/sun/tools/jcmd/jcmd-big-script.sh
No comment
Regards,
Fred
On 03/29/12 06:28 AM, David Buck wrote:
Hi!
Please review my fix for the following bug :
[ Bug ID: 7154822 forward port fix for Bug 13645891 to JDK8 jcmd (1024
byte file size limit issue) ]
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7154822
The issue is there is an arbitrary limit in the size of a script file
you pass to the jcmd command (via the -f option) of 1024 bytes. The
solution is for jcmd to break up the input file into individual lines
and send them one at a time to the jvm.
A similar fix has already been done for JRockit's jrcmd command and
will
be released in R28.2.3.
[ jdk ]
http://cr.openjdk.java.net/~dbuck/7154822/webrev.00/
All the default jprt tests and the jdk_tools tests were run and passed.
Cheers,
-Buck
