<< Back to previous view

[GLASSFISH-18948] The error message isn't friend to user while deploying a rar the size of which is larger than current allocated java heap size Created: 26/Jul/12  Updated: 10/Aug/12  Resolved: 10/Aug/12

Status: Resolved
Project: glassfish
Component/s: deployment
Affects Version/s: 4.0_b45
Fix Version/s: 4.0_b51

Type: Improvement Priority: Major
Reporter: Jeremy_Lv Assignee: Hong Zhang
Resolution: Fixed Votes: 0
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

Windows Xp


File Attachments: Zip Archive GLASSFISH-18948_revised_source.zip     JPEG File screenshot-1.jpg     JPEG File screenshot-2.jpg     Zip Archive test-output(QL_GF_profile).zip    
Tags:
Participants: Hong Zhang and Jeremy_Lv

 Description   

[Bug Description]
The error message isn't friend to user while deploying a rar the size of which is larger than current allocated java heap size

[Operations]
1、Prepare a rar which contains a large file(this file's size is larger than current allocated java heap size).

2、While deploying the rar using GUI or command, the following error message is displayed on cmd:

""Jave heap size""

[revise way]
let user know that the current deployed file is larger than current allocated java heap size.

[Affected versions ]
1 4.0_b45
2 gf's trunk until 2012/07/24



 Comments   
Comment by Jeremy_Lv [ 26/Jul/12 08:28 AM ]

The glassfish throw out the error messages(java heap space) defined in JDK on GUI.
This error message isn't friendly to user .

Comment by Jeremy_Lv [ 26/Jul/12 08:32 AM ]

I think the error messages should be more friendly , just as the picture shows

Comment by Jeremy_Lv [ 01/Aug/12 10:25 AM ]

I have uploaded my modified code to this improvement.

Comment by Hong Zhang [ 01/Aug/12 02:21 PM ]

Thanks for submitting the patch for this one. I think it's good that we report some error message for this, but is the cause of the error always related to heap size, could it be something else?

And as a general comment for this change and other changes, please make sure the changed files do not contain the windows character "^M" and the identation consistent with the existing code.

Comment by Jeremy_Lv [ 02/Aug/12 05:57 AM ]

The error message is only occured in the situation in deploying the application contains a large file which is larger than current allocated java heap size.

For example:
Deploy an application which contains a large file(this file's size is larger than current allocated java heap size). If your computer's RAM is 2G, I think the error messages(java heap size) can be thrown out by deploying an application contains a file larger than 700M.

I notice that copy#FileUtils execute the following code:

else{ ByteBuffer byteBuffer = ByteBuffer.allocate(Long.valueOf(size).intValue()); inChannel.read(byteBuffer); byteBuffer.rewind(); outChannel.write(byteBuffer); }

When excute the statement of "ByteBuffer byteBuffer = ByteBuffer.allocate(Long.valueOf(size).intValue());", you will find out the truth that the size may larger than the ByteBuffer can be allocated. After that, the error messages(Java heap size) are thrown out.

【My opition】
I think I should catch this error and throw out the messages like "Error while copying file to temp directory;the current deployed file is larger than current allocated Java heap space".

Comment by Jeremy_Lv [ 02/Aug/12 09:41 AM ]

Thanks for your advices, I have updated my modified files and sure about that the changed files don't contains windows character "^M".

Comment by Hong Zhang [ 02/Aug/12 03:28 PM ]

Maybe the try/catch block should be around the ByteBuffer.allocate call for this purpose, as there could be other exceptions thrown during reading and writing, and the error message would be misleading when it failed due to other purposes.
And for error message, please make the log string more specific, something like allocate.more.than.java.heap.space=Trying to allocate a buffer bigger than the current allocated Java heap space.

Thanks for fixing the ^Ms, please fix the code indentation also.

Comment by Jeremy_Lv [ 03/Aug/12 03:44 AM ]

Thanks for your comments.

I have modified the source and updated my attachment files.

The error messages are changed like allocate.more.than.java.heap.space=Trying to allocate a buffer bigger than the current allocated Java heap space.

By the way,the try/catch block has changed around the ByteBuffer.allocate call for this purpose.

Comment by Jeremy_Lv [ 03/Aug/12 03:45 AM ]

I'll run the QL tests and dev tests after you check the codes.

Comment by Hong Zhang [ 03/Aug/12 02:30 PM ]

Thanks, the changes look good. But can you fix the indentation, using the standard Java coding convention? For example, the catch line should move 4 spaces to the left to align with the try (and the code inside catch block needs to be fixed also), the last three lines of the method should align with try also?

Comment by Jeremy_Lv [ 06/Aug/12 01:41 AM ]

I'm sorry about that I haven't using the standard Java coding convention. Now
I have revised the codes as your comments.
Please check it again.
Best Regards.

Comment by Hong Zhang [ 06/Aug/12 03:47 PM ]

No problem and thanks for being so patient with me on the review comments

I looked at the changes, most of the indentations were fixed except the last line (the closing curly brace for the catch block), that line should be moved 4 spaces to the left.

Comment by Jeremy_Lv [ 07/Aug/12 03:08 AM ]

Thanks for being so kind about checking my changes.
I have fixed the indentation as your comments(the closing curly brace for the catch block).

Comment by Hong Zhang [ 07/Aug/12 02:28 PM ]

Changes look good. Please run the necessary tests before check in. And please check in all the files in the same svn commit as we discussed through the email.

Comment by Jeremy_Lv [ 08/Aug/12 10:28 AM ]

Dear Hong:
I have some dev tests questions:
for example:the modified file are related to the directory like "GF_main\nucleus\common\common-util\src\main\java\com\sun\enterprise\util\io", I can't find any profile files like "common" in the directory of "dev_test_main\devtests".
shall I run all of the dev tests?

Comment by Hong Zhang [ 08/Aug/12 03:51 PM ]

No, you don't need to do. As the fix you did was for a negative case and should not be harmful for regular cases, you could just run QuickLook tests.

Comment by Jeremy_Lv [ 09/Aug/12 02:24 AM ]

The QL tests results looks OK.

QL test information:
Test environment: Linux RHEL5-x32
Glassfish trunk: until 2012/08/06
QuickLook tests version: until 2012/08/06
scenarios: Glassfish Profile
Passed cases/Tests number: 107/107

I have uploaded my QL tests results.

Comment by Jeremy_Lv [ 09/Aug/12 02:26 AM ]

Sending main\nucleus\common\common-util\src\main\java\com\sun\enterprise\util\io\FileUtils.java
Sending main\nucleus\common\common-util\src\main\java\com\sun\enterprise\util\io\LocalStrings.properties
Transmitting file data ..
Committed revision 55370.

Generated at Sat Apr 19 21:10:39 UTC 2014 using JIRA 4.0.2#472.