1

I'm learning to use HikariCP (i'm new in java) and i found a wrapper but i think it's not thread safety, well the instance of the singleton is thread safety but not the method getConnection(). The class is this:

public class HikariCPWrapper{
    private static final HikariCPWrapper INSTANCE;
    private HikariDataSource ds;
    static  
    {  
    INSTANCE = new HikariCPWrapper();    
    }  

    private HikariCPWrapper(){
        HikariConfig config = new HikariConfig(); 
        //config.set... 
        //... 
        ds = new HikariDataSource(config);  

    }

    public static HikariCPWrapper getInstance ()  
    {  
        return INSTANCE;  
    }  

    public Connection getConnection()  throws SQLException  
    {  
        return ds.getConnection();  
    }  
}

Well, i've needed to send arguments to HikariConfig or HikariDataSource so I've re-write it this way:

public interface IConnectionProvider {
    void init(String jdbcUrl, String user, String password);
    Connection getConnection() throws SQLException;
}

    public class ConnectionProviderHikariCP implements IConnectionProvider{

    private static final ConnectionProviderHikariCP INSTANCE;
    private final HikariDataSource hikariDataSource;
    private Boolean initialized;
    //class initializer:
    static
    {
        INSTANCE = new ConnectionProviderHikariCP();
    }

    private ConnectionProviderHikariCP() {
        hikariDataSource = new HikariDataSource();
        initialized = false;
    }

    public static ConnectionProviderHikariCP getInstance() {
        return INSTANCE;
    }

    @Override
    public synchronized void init(String jdbcUrl, String user, String password) {
        hikariDataSource.setJdbcUrl(jdbcUrl);        
        hikariDataSource.setUsername(user);
        hikariDataSource.setPassword(password);
        initialized = true;
    }

    @Override
    public synchronized Connection getConnection() throws SQLException {
        if(!initialized)
           throw new UnsupportedOperationException("Debe inicializar mediante el método Init() primero!!!!!."); 

        return hikariDataSource.getConnection();
    }

}

I use it this way:

IConnectionProvider connectionProvider = ConnectionProviderHikariCP.getInstance();
connectionProvider.init(url, user, passwd);

BaseDAOFactory fatory = new MySqlDAOFactory(connectionProvider);
IExerciseBO exerciseBO = new ExerciseBO(fatory); 

But I'm not experienced in java so I need your advice. Is first class (the original) thread safety? Is my implementation thread safety?

AiApaec
  • 660
  • 1
  • 6
  • 12
  • 1
    Everything is thread-safe. I would change the code to make it respect Java naming conventions, though: mathods start with a lower-case letter, variables don't contain underscores. getConnection() should not throw Exception, but SQLException. The try/catch in the static block is useless, since the constructor doesn't throw any exception. – JB Nizet Apr 20 '15 at 05:24
  • @JBNizet thank you. So then Can i skip put syncronized in methods init() and getConnection()? (I'll update the code with your recomendatios). – AiApaec Apr 20 '15 at 06:00
  • 1
    No. Removing them would make the code not thread-safe. – JB Nizet Apr 20 '15 at 06:04
  • @JBNizet Oh I didn't understand your first comment. Thank you. – AiApaec Apr 20 '15 at 06:51

1 Answers1

4

HikariCP itself is thread-safe. As I understand it, what you are trying to do is prevent callers to getConnection() before the HikariDataSource has been initialized via the init() method. Presumably you also want to prevent multiple calls to init() the datasource.

Wrapping getConnection() with a synchronized is a bad idea in general with respect to pool throughput. I recommend the following pattern, based on your code above:

public class ConnectionProviderHikariCP implements IConnectionProvider {

  private static final ConnectionProviderHikariCP INSTANCE;
  private final HikariDataSource hikariDataSource;
  private AtomicBoolean initialized;
  //class initializer:
  static
  {
     INSTANCE = new ConnectionProviderHikariCP();
  }

  private ConnectionProviderHikariCP() {
     hikariDataSource = new HikariDataSource();
     initialized = new AtomicInteger();
  }

  public static ConnectionProviderHikariCP getInstance() {
     return INSTANCE;
  }

  @Override
  public void init(String jdbcUrl, String user, String password) {
     if (initialized.compareAndSet(false, true)) {
        hikariDataSource.setJdbcUrl(jdbcUrl);        
        hikariDataSource.setUsername(user);
        hikariDataSource.setPassword(password);
     }
     else {
        throw new IllegalStateException("Connection provider already initialized.");
     }
  }

  @Override
  public Connection getConnection() throws SQLException {
     if (initialized.get()) {
        return hikariDataSource.getConnection();
     }

     throw new UnsupportedOperationException("Debe inicializar mediante el método Init() primero!!!!!."); 
  }
}
brettw
  • 10,664
  • 2
  • 42
  • 59
  • "HikariCP itself is thread-safe." Where does this come from? If you are relying on an undocumented implementation detail please say so. – Martin Andersson Jan 18 '22 at 08:36