admin@glassfish.java.net

Re: CODE REVIEW: FindBugs: ASLauncher and DASLauncher

From: Byron Nevins <Byron.Nevins_at_Sun.COM>
Date: Tue, 27 Mar 2007 17:13:47 -0700

My immediate reaction is: NO! Don't commit this change. It is too
dangerous.

It's interesting that Findbugs is very concerned about writing to *one*
class variable from an instance, but it doesn't mind writing to the
"Super-Global-Statics" -- System.Properties, which this code does.
System Properties are hyper-thread-unsafe.

If you disagree then I need the new source file itself and ~ 4 hours to
thoroughly test it. IMHO, it isn't worth it...



Lloyd L Chambers wrote:
> Please review no later than noon on Thursday 3/30.
>
> Fixes a FindBugs complaint of "Write to static field".
>
> Lloyd
>
>
> RCS file:
> /cvs/glassfish/admin/servermgmt/src/java/com/sun/enterprise/admin/servermgmt/launch/ASLauncher.java,v
>
> retrieving revision 1.12
> diff -w -u -r1.12 ASLauncher.java
> ---
> admin/servermgmt/src/java/com/sun/enterprise/admin/servermgmt/launch/ASLauncher.java
> 16 Mar 2007 19:54:22 -0000 1.12
> +++
> admin/servermgmt/src/java/com/sun/enterprise/admin/servermgmt/launch/ASLauncher.java
> 27 Mar 2007 17:46:55 -0000
> @@ -154,7 +154,7 @@
> private static final String DOMAIN_NAME_SYSTEM_PROPERTY =
> "domain.name";
> private static final String SCRIPT_PATH_SYSTEM_PROPERTY =
> "com.sun.aas.scriptpath";
>
> - static int returnValue = 1;
> + int _returnValue = 1;
> String[] securityInfo; // username & passwords if neccessary...
>
> // WBN the level is here as a constant so that it can be changed
> to INFO or better
> @@ -176,9 +176,12 @@
>
> public static void main(String[] args)
> {
> + int returnValue = 1;
> +
> + ASLauncher pl=null;
> try
> {
> - ASLauncher pl=new ASLauncher();
> + pl=new ASLauncher();
> pl.preProcess(args); // figure out relocatable stuff
> pl.getSecurityFromUser(args);
> Process process = pl.process(args, pl.securityInfo);
> @@ -187,9 +190,21 @@
> {
> e.printStackTrace();
> }
> + finally
> + {
> + if ( pl != null )
> + {
> + returnValue = pl.getReturnValue();
> + }
> + }
> +
> System.exit(returnValue);
> }
>
> + int getReturnValue()
> + {
> + return _returnValue;
> + }
>
> /**
> * This method is meant to be called from the PLBootstrap.class
> to finish the setup
> @@ -335,7 +350,7 @@
> Process process = null;
> try
> {
> - returnValue=1;
> + _returnValue=1;
> securityInfo = SecurityInfo;
>
> if(securityInfo != null)
> @@ -379,7 +394,7 @@
> if (bDebug) System.out.println("ASLauncher Executing
> command ..");
>
> // if no exception, should have executed properly or
> logged errors
> - returnValue=0;
> + _returnValue=0;
>
> if(externalLogfileHandler != null)
> {
>
>
> RCS file:
> /cvs/glassfish/admin/servermgmt/src/java/com/sun/enterprise/admin/servermgmt/launch/DASLauncher.java,v
>
> retrieving revision 1.1
> diff -w -u -r1.1 DASLauncher.java
> ---
> admin/servermgmt/src/java/com/sun/enterprise/admin/servermgmt/launch/DASLauncher.java
> 18 Oct 2006 05:29:53 -0000 1.1
> +++
> admin/servermgmt/src/java/com/sun/enterprise/admin/servermgmt/launch/DASLauncher.java
> 27 Mar 2007 17:46:55 -0000
> @@ -35,7 +35,7 @@
> info.setSystemProps();
> asLauncher = new ASLauncher();
> asLauncher.process(args);
> - return asLauncher.returnValue;
> + return asLauncher._returnValue;
> }
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>