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
