So are you going to change this back? Ralph
On Aug 12, 2014, at 5:44 PM, Matt Sicker <boa...@gmail.com> wrote: > Oh my bad, I mixed up lazy singleton and eager singleton. > > > On 12 August 2014 17:43, Ralph Goers <ralph.go...@dslextreme.com> wrote: > 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.go...@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.go...@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.go...@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 <boa...@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 <garydgreg...@gmail.com> wrote: >>>>> Hi, >>>>> >>>>> What is the justification for this extra level? >>>>> >>>>> Gary >>>>> >>>>> >>>>> -------- Original message -------- >>>>> From: mattsic...@apache.org >>>>> Date:08/11/2014 22:04 (GMT-05:00) >>>>> To: comm...@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 <boa...@gmail.com> >>>> >>> >> > > > > > -- > Matt Sicker <boa...@gmail.com>
