Singleton Pattern problem
Hi,
I have a problem with Singleton Pattern, I have implemented Singleton Pattern for Connection Pool class for database, Singleton pattern is working perfectly fine. And I have another class(DatabaseConnection.java) with all database methods in it. Ideally, when I make some changes inDatabaseConnection class and upload to server, it should be still be able to use same old connection pool; instead a new connection pool is being created. i am not able to understand the mistake.
Heres my code
*****************************************************************************
CODE CONNECTION POOL CLASS:
import java.net.InetAddress;
import java.sql.*;
import java.util.*;
publicclass ConnectionPoolimplements Runnable{
private String driver, url, username, password;
privateint maxConnections, initialConnections;
privateboolean waitIfBusy;
private Vector availableConnections, busyConnections;
privateboolean connectionPending =false;
privatestatic ConnectionPool instance =null;
protected ConnectionPool(){
// Exists only to defeat instantiation.
}
publicstatic ConnectionPool getInstance()throws Exception{
if(instance ==null){
instance =new ConnectionPool();
instance.CreatePool();
}
return instance;
}
publicvoid CreatePool()
throws Exception{
boolean productionSite =false;
InetAddress iaddress = InetAddress.getLocalHost();
String hostname = iaddress.getHostName();
driver ="oracle.jdbc.driver.OracleDriver";
url ="XXXXXX";
username ="YYYY";
password ="ZZZZZ";
maxConnections = 5;
initialConnections = 5;
waitIfBusy =true;
if (initialConnections > maxConnections){
initialConnections = maxConnections;
}
availableConnections =new Vector(initialConnections);
busyConnections =new Vector();
for(int i=0; i<initialConnections; i++){
availableConnections.addElement(makeNewConnection());
}
}
publicsynchronized Connection getConnection()
throws SQLException{
if (!availableConnections.isEmpty()){
Connection existingConnection =
(Connection)availableConnections.lastElement();
int lastIndex = availableConnections.size() - 1;
availableConnections.removeElementAt(lastIndex);
if (existingConnection.isClosed()){
notifyAll();// Freed up a spot for anybody waiting
return(getConnection());
}else{
busyConnections.addElement(existingConnection);
return(existingConnection);
}
}else{
if ((totalConnections() >< maxConnections) &&
!connectionPending){
makeBackgroundConnection();
}elseif (!waitIfBusy){
thrownew SQLException("Connection limit reached");
}
try{
wait();
}catch(InterruptedException ie){}
return(getConnection());
}
}
privatevoid makeBackgroundConnection(){
connectionPending =true;
try{
Thread connectThread =new Thread(this);
connectThread.start();
}catch(OutOfMemoryError oome){
}
}
publicvoid run(){
try{
Connection connection = makeNewConnection();
synchronized(this){
availableConnections.addElement(connection);
connectionPending =false;
notifyAll();
}
}catch(Exception e){// SQLException or OutOfMemory
}
}
private Connection makeNewConnection()
throws SQLException{
try{
Class.forName(driver);
Connection connection =
DriverManager.getConnection(url, username, password);
return(connection);
}catch(ClassNotFoundException cnfe){
thrownew SQLException("Can't find class for driver: " +
driver);
}
}
publicsynchronizedvoid free(Connection connection){
busyConnections.removeElement(connection);
availableConnections.addElement(connection);
notifyAll();
}
publicsynchronizedint totalConnections(){
return(availableConnections.size() +
busyConnections.size());
}
publicsynchronizedvoid closeAllConnections(){
closeConnections(availableConnections);
availableConnections =new Vector();
closeConnections(busyConnections);
busyConnections =new Vector();
}
privatevoid closeConnections(Vector connections){
try{
for(int i=0; i<connections.size(); i++){
Connection connection =
(Connection)connections.elementAt(i);
if (!connection.isClosed()){
connection.close();
}
}
}catch(SQLException sqle){
// Ignore errors; garbage collect anyhow
}
}
publicsynchronized String toString(){
InetAddress iaddress =null;
String hostname ="";
try{
iaddress = InetAddress.getLocalHost();
hostname = iaddress.getHostName();
}catch(Exception e){
hostname ="";
}
String info =
"ConnectionPool(" + url +"," + username +")" +
", available=" + availableConnections.size() +
", busy=" + busyConnections.size() +
", max=" + maxConnections +
", IM=" + hostname;
return(info);
}
protectedvoid finalize(){
this.closeAllConnections();
log.store("ConnectionPool: finalize","Finalizing Connection Pool Object");
}
}
*****************************************************************************
CODE FOR DATABASE METHODS
import java.io.File;
import java.sql.*;
import java.util.Calendar;
import java.util.HashMap;
publicclass DatabaseConnection{
privatestatic ConnectionPool cp;
private Connection getConnection()throws Exception{
if (cp !=null){
return (cp.getConnection());
}else{
cp = ConnectionPool.getInstance();
return(cp.getConnection());
}
}
publicsynchronized String getBulletinID()throws Exception{
ResultSet rs =null;
String zeros ="000000";
int count = 0;
Connection con =null
try{
con = this.getConnection();
String query ="SELECT COUNT(*) FROM CA_RMA_BULLETIN";
PreparedStatement preStmt = con.prepareStatement(query);
rs = preStmt.executeQuery();
rs.next();
count = rs.getInt(1);
}finally{
rs.close();
preStmt.close();
cp.free(con);
}
return ("WT"+ zeros.substring(0, 6 - String.valueOf(count + 1).length()) + String.valueOf(count + 1));
}
*****************************************************************************
CODE THE WAY I CALL DATABASE METHODS:
DatabaseConnection db =new DatabaseConnect();
String s = db.getBulletinID();
Thanks in advance>
"Upload to server" means what exactly?If you have a running server and you are inserting a new class into that server then there MUST be two of them.If you are bouncing the server and there are still two then you are delivering it incorrectly.
Upload Server means, copying class to application server's classes folder.
Yes, If i bounce the server, everything works fine. but without bouncing the server, if i just copy the DatabaseConnection.class and not ConnectionPool.class, how come a new connection pool is created, isn't it, it supposed to use same old connection pool?
> Upload Server means, copying class to application
> server's classes folder.
> without bouncing the server, if i just copy the
> DatabaseConnection.class and not
> ConnectionPool.class, how come a new connection pool
> is created, isn't it, it supposed to use same old
> connection pool?
If you just copy the new class file, the VM per itself has no reason to discard the old class and load the new one instead. So if the server hotloads your new version of the class, it means it uses some custom mechanism, based on a different class loader instance.
Then the new class loader probably also reloads the ConnectionPool class, and both ConnectionPool classes, loaded by 2 different class loaders, are different (even if the class file is the same). And the static singleton instance is initialized in the new class.
Moral : such a Singleton is only a Singleton within a given class loader space.
Plus, it's not that good a singleton anyway. It can easily be defeated by subclassing.
OP:
1) Make the constructor private or anybody can subclass your singleton and create as many as they want
2) Your naming convention is a bit screwy on CreatePool()
3) You should probably make getInstance() synchronized.
> Upload Server means, copying class to application
> server's classes folder.
>
> Yes, If i bounce the server, everything works fine.
> but without bouncing the server, if i just copy the
> DatabaseConnection.class and not
> ConnectionPool.class, how come a new connection pool
> is created, isn't it, it supposed to use same old
> connection pool?
As I said then there must be two of them.
Hotloading usually (if done correctly) discards the older version once it is no longer in use. Of course if done incorrectly then both versions are maintained forever.
And as long as one version is in use nothing that any programmer will do will allow it to be discarded. Which is a very good thing.
Since you are declaring the ConnectionPool as cp in your database methods class. When ever you update the class and reload this gets initialized to null and so it gets the istance of singleton which n turn creates new connection pool.I think we cant avoid this.
> Since you are declaring the ConnectionPool as cp in
> your database methods class. When ever you update the
> class and reload this gets initialized to null and so
> it gets the istance of singleton which n turn creates
> new connection pool.
>
> I think we cant avoid this.
Hotloading (reloading) means that two instances of the singleton can exist at the same time.
If one unloads the first one before loading the second then that could be prevented. In an environment that requires hotloading in the first place that probably isn't going to be a good idea though.
> I think we cant avoid this.
i think you can, but it's not easy:
- you'll have to write your own classloader
- upon reload you'll have to inalidate the old instance
- you'll have to have a proxy-object for ConnectionPool which checks for the invalidation on every access to any method (in case of invalidation will switch to the new pool)
this is just theoretical. i have no idea if/how the various app-servers are supporting/allowing this.
> > I think we cant avoid this.
>
> i think you can, but it's not easy:
> - you'll have to write your own classloader
By definition hotloading requires this.
> - upon reload you'll have to inalidate the old
> instance
> - you'll have to have a proxy-object for
> ConnectionPool which checks for the invalidation on
> every access to any method (in case of invalidation
> will switch to the new pool)
I don't believe that is the problem. And if you can prevent access to the original object in the first place then the problem is solved anyways.
> > - you'll have to write your own classloader
>
> By definition hotloading requires this.
as we might be talking about servlets the application server might normaly do the hotloading...
> I don't believe that is the problem. And if you can
what do you think is the problem?
i think it's that the appserver hotloads the new class and the appserver can not trash the old class and/or instance, maybe because the singleton instance of the old class still exists, maybe because of other problems (AFAIK tomcat under windows has problems with redeploying)
> prevent access to the original object in the first
> place then the problem is solved anyways.
solving the problem was my intent :-D
serious: the selfmade classloader can check for reloading of ConnectionPool and in this case do a :
proxyConnectionPool.setStaticConPool(new ConnectionPool)
after that the instance of the old class is no longer in use and it doesn't even matter if the class can be unloaded.
all owners of the ProxyConnectionPool will use the new implementation...
cheers
raudi
ps: i'm not talking about "java.lang.reflect.Proxy" im talking about the GoF pattern "proxy"
http://www.dofactory.com/Patterns/PatternProxy.aspx
PPS:
don't get me wrong: i'm not saying this is an elegant solution!
i'm just saying that if it is necessary it might be a way to implement it.
in a production server i would never want a mechanism that changes my connection pooling by dynamically reloading of a class.
i'd have several connection pooling implementations that implement the same interface and a mechanism to change the used mechanism.
> > > - you'll have to write your own classloader
> >
> > By definition hotloading requires this.
>
> as we might be talking about servlets the application
> server might normaly do the hotloading...
Huh?
Hotloading is hotloading. If the app does it then it does it.
>
> > I don't believe that is the problem. And if you
> can
>
> what do you think is the problem?
> i think it's that the appserver hotloads the new
> class and the appserver can not trash the old class
> and/or instance, maybe because the singleton instance
> of the old class still exists, maybe because of other
> problems (AFAIK tomcat under windows has problems
> with redeploying)
There are two parts to this.
1. The VM can not collect an instance until there are no more active references to it.
2. If a hotloading is written correctly then once the new version is loaded, by definitition, there will at some time no longer be any active references to the old version.
Those are the only things that impact whether an older instance still exists or not.
If the hotloader is not written correctly then there are any number of variations that will end up keeping active references to the older version and thus it will never be collected.
>
> > prevent access to the original object in the first
> > place then the problem is solved anyways.
>
> solving the problem was my intent :-D
>
> serious: the selfmade classloader can check for
> reloading of ConnectionPool and in this case do a :
> proxyConnectionPool.setStaticConPool(new
> ConnectionPool)
>
> after that the instance of the old class is no longer
> in use and it doesn't even matter if the class can be
> unloaded.
>
Errr...yes it does. If there is a pool then the pool will hold active connections. And quite possibly those connections will not be disposed of until the pool is collected. And that can be very important.
> > > > - you'll have to write your own classloader
> > >
> > > By definition hotloading requires this.
> >
> > as we might be talking about servlets the
> application
> > server might normaly do the hotloading...
>
> Huh?
>
> Hotloading is hotloading. If the app does it then it
> does it.
please read all posts...
i say hotloading is normaly done by the appserver.
i say that the problem might be solved by implementing his own additional hotloading.
> There are two parts to this.
> 1. The VM can not collect an instance until there are
> no more active references to it.
that's exactly what i'm talking about. he should implement his own classloader to make sure he gets notified when the class is reloaded.
> 2. If a hotloading is written correctly then once the
> new version is loaded, by definitition, there will at
> some time no longer be any active references to the
> old version.
"some time" is not good enough. the users of the pool will not be informed that the class was reloaded. so there will be references for quite a long time...
> > in use and it doesn't even matter if the class can
> be
> > unloaded.
> >
>
> Errr...yes it does. If there is a pool then the pool
> will hold active connections. And quite possibly
> those connections will not be disposed of until the
> pool is collected. And that can be very important.
you are right here. but i wrote he should "in(v)alidate the old instance" (sorry for the lost 'v' :-} ) it seems natural to me that this would include closing any ressources. and after this is done it wouldn't matter what the VM does with the old class and instance.
in short: what i say is that
1) he should implement a notification mechanism that is used to inform about the reloading of the class (AFAIK there is no such mechanism in java or tomcat)
2) he should use the proxy pattern so that he can keep the existing classes as they are and that he can handle the reloading in one central location
and
3) i say that i personaly would not do this on a production system. i don't see the need for something like that:
- changing of pooling strategies should be done by having several implementations of an interface and switching between these classes.
- updating/bugfixing etc. of a single strategy should preferably be done (IMO) by restarting the application if not the appserver.
any ideas for better or worse ;-) solutions?
reply 11 maybe relevant, http://forum.java.sun.com/thread.jspa?threadID=712389&tstart=0
>
> > 2. If a hotloading is written correctly then once the
> > new version is loaded, by definitition, there will at
> > some time no longer be any active references to the
> > old version.
>
> "some time" is not good enough. the users of the pool
> will not be informed that the class was reloaded. so
> there will be references for quite a long time...
Best you can do. In java you can not control when the VM decides that a class is unloadable.
>
> > 2. If a hotloading is written correctly then once the
> > new version is loaded, by definitition, there will at
> > some time no longer be any active references to the
> > old version.
>
> "some time" is not good enough. the users of the pool
> will not be informed that the class was reloaded. so
> there will be references for quite a long time...
> Best you can do. In java you can not control when
> the VM decides that a class is unloadable.
no, not best you can do!
it's probably true that there is no mechanism
that tells you when a class is unloaded
*but* we will not have to care about that because we are
eliminating all references to all instances of the old
class ourselfs:
class MyClassloader {
defineClass(..) {
...
if (name == "MyRessourcePool") {
MyProxyResPool.invalidateResPool()
}
...
}
...
}
class MyProxyResPool {
static MyRessourcePool thePool = null;
static MyProxyResPool theInstance = new MyProxyResPool();
public getInstance() {
return theInstance;
}
public invalidateResPool() {
thePool=null;
}
private getPool() {
if (thePool == null) {
thePool = MyRessourcePool.getInstance();
}
return thePool;
}
public anyAction(Any any) {
getPool().anyAction(any);
}
public anotherAction(Any any) {
getPool().anotherAction(any);
}
...
}
class ResPoolClient {
public whatever() {
MyProxyResPool.getInstance.anyAction(whatever);
}
}
raudia at 2007-7-20 21:30:40 >
> >
> > > 2. If a hotloading is written correctly then once the
> > > new version is loaded, by definitition, there will at
> > > some time no longer be any active references to the
> > > old version.
> >
> > "some time" is not good enough. the users of the pool
> > will not be informed that the class was reloaded. so
> > there will be references for quite a long time...
>
> > Best you can do. In java you can not control when
> > the VM decides that a class is unloadable.
>
> no, not best you can do!
> it's probably true that there is no mechanism
> that tells you when a class is unloaded
Which is what I said.
>
> *but* we will not have to care about that because we are
> eliminating all references to all instances of the old
> class ourselfs:
I never claimed otherwise.
I suggest you re-read what I wrote.
