logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ralph Goers <ralph.go...@dslextreme.com>
Subject Re: svn commit: r1617397 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java
Date Tue, 12 Aug 2014 22:40:42 GMT
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 <ralph.goers@dslextreme.com> 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 <ralph.goers@dslextreme.com> 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 <boards@gmail.com> 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 <garydgregory@gmail.com> wrote:
>>> Hi,
>>> 
>>> What is the justification for this extra level?
>>> 
>>> Gary
>>> 
>>> 
>>> -------- Original message --------
>>> From: mattsicker@apache.org
>>> Date:08/11/2014 22:04 (GMT-05:00)
>>> To: commits@logging.apache.org
>>> 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 <boards@gmail.com>
>> 
> 


Mime
View raw message