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:43:22 GMT
Finally, see http://en.wikipedia.org/wiki/Singleton_pattern#Eager_initialization.  This was
the prior pattern which is clearly thread safe.

Ralph

On Aug 12, 2014, at 3:40 PM, Ralph Goers <ralph.goers@dslextreme.com> wrote:

> 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