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 <[email protected]> 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 <[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]> >>> >> >
