Re: svn commit: r1617397 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java
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=1617397r1=1617396r2=1617397view=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 ListColumn 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
Re: RE: svn commit: r1617397 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java
On Wednesday, 13 August 2014, Ralph Goers ralph.go...@dslextreme.com
wrote:
So are you going to change this back?
Yeah I'll revert it unless someone else snags it first. I won't be at a
computer for several hours, though.
I would note that this is my relic of old Java development. Similar to the
calls of size() 0 instead of isEmpty() you still find all over the place.
Ralph
On Aug 12, 2014, at 5:44 PM, Matt Sicker boa...@gmail.com
javascript:_e(%7B%7D,'cvml','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
javascript:_e(%7B%7D,'cvml','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
javascript:_e(%7B%7D,'cvml','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
javascript:_e(%7B%7D,'cvml','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
javascript:_e(%7B%7D,'cvml','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
javascript:_e(%7B%7D,'cvml','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
javascript:_e(%7B%7D,'cvml','garydgreg...@gmail.com'); wrote:
Hi,
What is the justification for this extra level?
Gary
Original message
From: mattsic...@apache.org
javascript:_e(%7B%7D,'cvml','mattsic...@apache.org');
Date:08/11/2014 22:04 (GMT-05:00)
To: comm...@logging.apache.org
javascript:_e(%7B%7D,'cvml','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=1617397r1=1617396r2=1617397view=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 ListColumn columns;
private final ConnectionSource connectionSource;
@@ -174,10 +173,19 @@ public final class JdbcDatabaseManager e
Re: RE: svn commit: r1617397 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java
On Wed, Aug 13, 2014 at 1:31 PM, Matt Sicker boa...@gmail.com wrote:
On Wednesday, 13 August 2014, Ralph Goers ralph.go...@dslextreme.com
wrote:
So are you going to change this back?
Yeah I'll revert it unless someone else snags it first. I won't be at a
computer for several hours, though.
Matt, please do undo when you do get a chance.
Gary
I would note that this is my relic of old Java development. Similar to the
calls of size() 0 instead of isEmpty() you still find all over the place.
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=1617397r1=1617396r2=1617397view=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 ListColumn 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
Re: RE: svn commit: r1617397 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java
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=1617397r1=1617396r2=1617397view=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 ListColumn 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
Re: svn commit: r1617397 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java
It's a rare problem, but if two threads attempt to initialize a class for
the first time, two instances of the singleton may end up being created.
However, using this pattern, the only possible access point to create the
singleton is through the Holder class which will only be called once in
this pattern.
On 12 August 2014 15:13, 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=1617397r1=1617396r2=1617397view=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 ListColumn 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
Re: svn commit: r1617397 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java
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=1617397r1=1617396r2=1617397view=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 ListColumn 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
Re: svn commit: r1617397 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java
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=1617397r1=1617396r2=1617397view=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 ListColumn 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
Re: svn commit: r1617397 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java
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=1617397r1=1617396r2=1617397view=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 ListColumn 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
Re: svn commit: r1617397 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcDatabaseManager.java
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=1617397r1=1617396r2=1617397view=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 ListColumn 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
