Let me put it another way.  

If your intent is that the factory should only be created when getManager is 
called vs when AbstractDataManager is loaded then your change is fine.  
However, I am not sure there is any benefit to that since I would expect every 
access to AbstractDataManager to be immediately followed by a call to 
getManager.  

From what I can see the previous code was not vulnerable to any race conditions.

Ralph

On Aug 12, 2014, at 3:33 PM, Ralph Goers <[email protected]> wrote:

> So you are saying static initializers in a class can be executed multiple 
> times?  If so I certainly wasn’t aware of that and I would think a few pieces 
> of code would need modification.  Before I go searching can you point to 
> something that says it works that way? 
> 
> When I read 
> http://en.wikipedia.org/wiki/Initialization-on-demand_holder_idiom it 
> indicates that static initialization IS guaranteed to be serial, in which 
> case the holder pattern is not needed for this.  That pattern is for when you 
> want lazy initialization, not for guaranteeing a static variable is only 
> initialized once.
> 
> Ralph
> 
> On Aug 12, 2014, at 1:13 PM, Ralph Goers <[email protected]> wrote:
> 
>> What race condition?  Static variables are initialized when the class is 
>> constructed.  As far as I know there is no race condition on that or Java 
>> would be hosed.
>> 
>> Ralph
>> 
>> On Aug 12, 2014, at 12:07 PM, Matt Sicker <[email protected]> wrote:
>> 
>>> Prevents multiple threads from constructing the field if they access the 
>>> class concurrently. Basically, it prevents a race condition. The 
>>> alternative is to make the field volatile and do a double-checked lock 
>>> which we do in another class somewhere.
>>> 
>>> 
>>> On 12 August 2014 13:53, Gary Gregory <[email protected]> wrote:
>>> Hi,
>>> 
>>> What is the justification for this extra level?
>>> 
>>> Gary
>>> 
>>> 
>>> -------- Original message --------
>>> From: [email protected]
>>> Date:08/11/2014 22:04 (GMT-05:00)
>>> To: [email protected]
>>> Subject: svn commit: r1617397 - 
>>> /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java
>>> 
>>> Author: mattsicker
>>> Date: Tue Aug 12 02:04:38 2014
>>> New Revision: 1617397
>>> 
>>> URL: http://svn.apache.org/r1617397
>>> Log:
>>> Singleton pattern
>>> 
>>> Modified:
>>>     
>>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java
>>> 
>>> Modified: 
>>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java
>>> URL: 
>>> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java?rev=1617397&r1=1617396&r2=1617397&view=diff
>>> ==============================================================================
>>> --- 
>>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java
>>>  (original)
>>> +++ 
>>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java
>>>  Tue Aug 12 02:04:38 2014
>>> @@ -36,7 +36,6 @@ import org.apache.logging.log4j.core.uti
>>>   * An {@link AbstractDatabaseManager} implementation for relational 
>>> databases accessed via JDBC.
>>>   */
>>> public final class JdbcDatabaseManager extends AbstractDatabaseManager {
>>> -    private static final JDBCDatabaseManagerFactory FACTORY = new 
>>> JDBCDatabaseManagerFactory();
>>> 
>>>      private final List<Column> columns;
>>>      private final ConnectionSource connectionSource;
>>> @@ -174,10 +173,19 @@ public final class JdbcDatabaseManager e
>>>                                                               final 
>>> ColumnConfig[] columnConfigs) {
>>> 
>>>          return AbstractDatabaseManager.getManager(
>>> -                name, new FactoryData(bufferSize, connectionSource, 
>>> tableName, columnConfigs), FACTORY
>>> +                name, new FactoryData(bufferSize, connectionSource, 
>>> tableName, columnConfigs), getFactory()
>>>          );
>>>      }
>>> 
>>> +    // the usual lazy singleton
>>> +    private static class Holder {
>>> +        private static final JDBCDatabaseManagerFactory INSTANCE = new 
>>> JDBCDatabaseManagerFactory();
>>> +    }
>>> +
>>> +    private static JDBCDatabaseManagerFactory getFactory() {
>>> +        return Holder.INSTANCE;
>>> +    }
>>> +
>>>      /**
>>>       * Encapsulates data that {@link JDBCDatabaseManagerFactory} uses to 
>>> create managers.
>>>       */
>>> 
>>> 
>>> 
>>> 
>>> 
>>> -- 
>>> Matt Sicker <[email protected]>
>> 
> 

Reply via email to