Issue Details (XML | Word | Printable)

Key: GLASSFISH-16307
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Hong Zhang
Reporter: Dies Koper
Votes: 0
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
glassfish

NPE and issues when deploying applications with same name - different case

Created: 02/Apr/11 06:50 PM   Updated: 11/Sep/12 09:58 AM   Resolved: 11/Sep/12 09:58 AM
Component/s: deployment
Affects Version/s: 3.1
Fix Version/s: 4.0_b53

Time Tracking:
Not Specified

File Attachments: 1. Zip Archive GLASSFISH-16307_FIXED_SOURCE.zip (15 kB) 10/Sep/12 02:22 AM - Jeremy_Lv

Environment:

WinXP


Tags: 3_1_1-scrubbed 3_1_2-exclude
Participants: Dies Koper, Hong Zhang, Jeremy_Lv, scatari and Tim Quinn


 Description  « Hide

I deploy my app 'web25.war' using the CUI. Later I deploy another app 'WEB25.war'. No errors in console or server log, but I noticed domains\domain1\applications only has a directory WEB25, no web25.
I undeploy 'web25', again seemingly successful. But when I then try to disable WEB25 I get an error:

D:\GFv3.1\glassfish-3.1\glassfish3\glassfish>bin\asadmin disable
Enter the value for the name operand> WEB25
remote failure:
Command disable failed.

In server.log an NPE:

[#|2011-04-03T11:41:31.796+1000|SEVERE|glassfish3.1|javax.enterprise.system.tools.admin.org.glassfish.deployment.admin|_ThreadID=56;_ThreadName=Thread-1;|Error during disabling:
java.lang.NullPointerException
at com.sun.enterprise.deploy.shared.FileArchive.getDirectories(FileArchive.java:175)
at org.glassfish.deployment.common.DeploymentUtils.isEARFromIntrospecting(DeploymentUtils.java:332)
at org.glassfish.deployment.common.DeploymentUtils.isEAR(DeploymentUtils.java:320)
at org.glassfish.javaee.full.deployment.EarHandler.handles(EarHandler.java:146)
[...]
[#|2011-04-03T11:42:22.218+1000|WARNING|glassfish3.1|javax.enterprise.system.tools.admin.org.glassfish.deployment.admin|_ThreadID=82;_ThreadName=Thread-1;|Cannot find application bits at D:\tests\GFv3.1\glassfish-3.1\glassfish3\glassfish\domains\domain1\applications\WEB25|#]

If deployment of apps only differing in case is not supported it would be better not to allow it.



Hong Zhang added a comment - 04/Apr/11 06:36 AM

I could not reproduce the issue yet (I was trying with the trunk build). I tried both CLI and GUI, things worked as expected in either case:
1. deploy web25.war and WEB25.war
both deployments successful, and the applications repository have both directories
2. undeploy web25
undeployment successful
3. disable WEB25
disable successful

Maybe it's something specific to your test cases? Could you attach the test cases you tried and also if possible trying with trunk build to see if it makes any difference?


Hong Zhang added a comment - 04/Apr/11 06:40 AM

I just realized you were trying on windows. So this could be a windows specific issue.


Tim Quinn added a comment - 04/Apr/11 08:16 AM

As Hong suspected this is happening because you are on Windows XP which treats two file paths as the same even if they have different case.

GlassFish itself treats the two applications separately because they have different applicaitons names (web25 and WEB25). But the file system treats the web25 and WEB25 directories in ${domainDir}/applications as the same.

After you undeploy one application the ${domainDir}/applications/WEB25 directory is gone, but that is where GlassFish expects to find the files for web25 which, as you noted, causes problems.

We'll need to discuss the right fix for this. One possibility is for GlassFish to reject the deployment of a second app that would share the same directory as a previously-deployed app. This would happen only on Windows and only for archive deployments and only if the directory for the new app in the applications directory would be the same as the directory for an existing app.


scatari added a comment - 04/Nov/11 09:55 PM

Please evaluate this for possible inclusion into 3.1.2.


Hong Zhang added a comment - 14/Dec/11 05:47 PM

Could not get to it in this release, defer it to next release as it's a corner case.


Jeremy_Lv added a comment - 09/Aug/12 07:06 AM

Dear Hong,Tim:

We'll need to discuss the right fix for this. One possibility is for GlassFish to reject the deployment of a second app that would share the same directory as a previously-deployed app. This would happen only on Windows and only for archive deployments and only if the directory for the new app in the applications directory would be the same as the directory for an existing app.

I'll investigate this issue, Does the fix for GlassFish is just to reject the deployment of a second app that would share the same directory as a previously-deployed app? I'll offer the patch in few days.


Hong Zhang added a comment - 09/Aug/12 02:48 PM

Yeah, I guess that could be the fix for this corner case. And the error message in addition to say the directory name is already being used, could also suggest the user to use a different application name to avoid conflict. This check could be made in ApplicationLifecycle.getContext method after this:

File expansionDir = new File(domain.getApplicationRoot(),
repositoryBitName);


Jeremy_Lv added a comment - 10/Aug/12 10:02 AM

Dear Hong:
Thanks for your sugguestions, Anyway, I think we should prevent the first directory to be deleted by follow code path on the environment of windows:
DeployCommand#excute

if (reposDir.exists()) {
    /*
    * Delete the repository directory as an archive to allow
    * any special processing (such as stale file handling)
    * to run.
    */
    final FileArchive arch = DeploymentUtils.openAsFileArchive(reposDir, archiveFactory);
    arch.delete();
}

I'll continue to investigate this issue and upload my patch after my vocation finished.


Hong Zhang added a comment - 10/Aug/12 02:40 PM

You are right, the code you pointed above does happen before ApplicationLifecycle.getContext call. So we should do the check here and reject the deployment as needed. One thing we need to be careful here is there are valid cases where we need to clean up any left over directories from last deployment, so probably the check here would be if the reposDir is same, but the app name is different, then we fail deployment.


Jeremy_Lv added a comment - 21/Aug/12 08:48 AM

Dear Hong:
I have revised this issue and uploaded my revised source files.
please check it and give me some advices.


Hong Zhang added a comment - 29/Aug/12 01:51 PM

We should avoid introducing platform dependent checks in the code. The check should be if the respository directory is the same as a deployed application (for each application element in domain.xml, there is a location URI for where the repository is, you could use new File(new URI(application.getLocation()) to get the repository dir to compare), and the application name is not the same, fail the deployment.


Jeremy_Lv added a comment - 31/Aug/12 06:54 AM

Dear Hong:
I have updated my attachments, please review my code and give me some advices.


Hong Zhang added a comment - 05/Sep/12 02:00 PM

The check should include both name and repository dir, if the name does not match, but repository match, that's this reported use case and we want to fail deployment. If the name and repository both match, that's the normal case where we do want to do the clean up for any left over directories.


Jeremy_Lv added a comment - 06/Sep/12 03:02 AM

I see. I have fixed it again and uploaded to the JIRA.
Please check it again.
Thanks


Hong Zhang added a comment - 06/Sep/12 04:06 PM

A couple comments on the new diffs:

1. For the if block, I think we should compare the path of the repository directories instead of the File object itself (e.g. you can use File.getAbsolutePath() to compare). I don't think we should use an else if block here, you should use the else block for everything else, and we will delete the left over directories. We only want to limit the change to this particular corner use case, and everything else should stay same.

2. For the error message, please say something like Application <the name of the new app> is trying to use the same repository directory as application <the one that's already deployed>, please choose a different application name to deploy.


Hong Zhang added a comment - 06/Sep/12 04:11 PM

And please run these two manual tests to make sure things work fine, the one reported in this issue, and the one where we do need to do clean up and the deployment should proceed (you could manually create some left over contents in the repository and see if the deployment with the right application name will clean those up as expected in the beginning of the deployment lifecycle).


Jeremy_Lv added a comment - 10/Sep/12 02:22 AM

Hong:
I have fixed the source again under your suggestion.please check it.

BTW:

And please run these two manual tests to make sure things work fine, the one reported in this issue, and the one where we do need to do clean up and the deployment should proceed (you could manually create some left over contents in the repository and see if the deployment with the right application name will clean those up as expected in the beginning of the deployment lifecycle).

I have run these two manual tests and it seems fine.


Hong Zhang added a comment - 10/Sep/12 02:50 PM - edited

The latest changes look pretty good, please go ahead and check in (after running the usual set of the tests).


Jeremy_Lv added a comment - 11/Sep/12 09:58 AM

all QL tests and dev tests passed according to the latest version of GF.
Sending nucleus\deployment\admin\src\main\java\org\glassfish\deployment\admin\DeployCommand.java
Sending nucleus\deployment\admin\src\main\java\org\glassfish\deployment\admin\LocalStrings.properties
Transmitting file data ..
Committed revision 55893.