Yes

While walking the dog I realized that the change isn't good - the 
synchronization should be on a lock object such as:

Private static final Object lock = new Object()

Or using some of the fancy latch stuff from JDK5

I am at work, Willem could you revert the change?

Med venlig hilsen
 
Claus Ibsen
......................................
Silverbullet
Skovsgårdsvænget 21
8362 Hørning
Tlf. +45 2962 7576
Web: www.silverbullet.dk
-----Original Message-----
From: Willem Jiang [mailto:[EMAIL PROTECTED] 
Sent: 28. maj 2008 08:47
To: [EMAIL PROTECTED]
Subject: Re: svn commit: r660811 - 
/activemq/camel/trunk/camel-core/src/main/java/org/apache/camel/util/UuidGenerator.java

Hi Claus,

I don't think the below codes is enough safe, as you may call the 
synchronized for a null object reference.

private static String UNIQUE_STUB;

public UuidGenerator(String prefix) {
        synchronized (UNIQUE_STUB) {
            if (UNIQUE_STUB == null) {
                init();
            }
            this.seed = prefix + UNIQUE_STUB + (instanceCount++) + "-";
        }
    }

BTW, there are lots of wired exceptions appear in the Banboo[1] after 
you committed the blow change.
Could you revert it ?

[1]http://bamboo.logicblaze.com:8085/browse/CAMEL-TRUNK-274

Willem

[EMAIL PROTECTED] wrote:
> Author: davsclaus
> Date: Tue May 27 21:56:11 2008
> New Revision: 660811
>
> URL: http://svn.apache.org/viewvc?rev=660811&view=rev
> Log:
> UUidGenerator not using static initializer codeblock to avoid class 
> initialization problems (if they fail then they are never reported = kept in 
> the dark)
>
> Modified:
>     
> activemq/camel/trunk/camel-core/src/main/java/org/apache/camel/util/UuidGenerator.java
>
> Modified: 
> activemq/camel/trunk/camel-core/src/main/java/org/apache/camel/util/UuidGenerator.java
> URL: 
> http://svn.apache.org/viewvc/activemq/camel/trunk/camel-core/src/main/java/org/apache/camel/util/UuidGenerator.java?rev=660811&r1=660810&r2=660811&view=diff
> ==============================================================================
> --- 
> activemq/camel/trunk/camel-core/src/main/java/org/apache/camel/util/UuidGenerator.java
>  (original)
> +++ 
> activemq/camel/trunk/camel-core/src/main/java/org/apache/camel/util/UuidGenerator.java
>  Tue May 27 21:56:11 2008
> @@ -28,14 +28,30 @@
>   */
>  public class UuidGenerator {
>  
> -    private static final transient Log LOG = 
> LogFactory.getLog(UuidGenerator.class); 
> -    private static final String UNIQUE_STUB;
> +    private static final transient Log LOG = 
> LogFactory.getLog(UuidGenerator.class);
> +    private static String UNIQUE_STUB;
>      private static int instanceCount;
>      private static String hostName;
>      private String seed;
>      private long sequence;
>  
> -    static {
> +    public UuidGenerator(String prefix) {
> +        synchronized (UNIQUE_STUB) {
> +            if (UNIQUE_STUB == null) {
> +                init();
> +            }
> +            this.seed = prefix + UNIQUE_STUB + (instanceCount++) + "-";
> +        }
> +    }
> +
> +    public UuidGenerator() {
> +        this("ID-" + hostName);
> +    }
> +
> +    /**
> +     * Initializes the stub - not static codeblock to let commons-logging 
> work
> +     */
> +    private void init() {
>          String stub = "";
>          boolean canAccessSystemProps = true;
>          try {
> @@ -61,19 +77,14 @@
>              hostName = "localhost";
>              stub = "-1-" + System.currentTimeMillis() + "-";
>          }
> +
>          UNIQUE_STUB = stub;
> -    }
>  
> -    public UuidGenerator(String prefix) {
> -        synchronized (UNIQUE_STUB) {
> -            this.seed = prefix + UNIQUE_STUB + (instanceCount++) + "-";
> +        if (LOG.isDebugEnabled()) {
> +            LOG.debug("UuidGenerator initialized with stub genereated: " + 
> UNIQUE_STUB);
>          }
>      }
>  
> -    public UuidGenerator() {
> -        this("ID-" + hostName);
> -    }
> -
>      /**
>       * As we have to find the hostname as a side-affect of generating a 
> unique
>       * stub, we allow it's easy retrevial here
>
>
>
>   

Reply via email to