Should I synchronize getConnection() of DataSource

Question

I have this singelton with Connection Pool.

public enum ConnectionPool {
    INSTANCE;

    private DataSource ds = null;

    ConnectionPool() {
      try {
         final Context initCtx = new InitialContext();
         ds = (DataSource) initCtx.lookup("java:comp/env/jdbc/test");
      } catch (NamingException e) {
         e.printStackTrace();
      }
    }

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

Should I synchronize getConnection() in order to prevent getting the same instance from different threads? It looks like it has been already synchronized by Tomcat but I'm not sure(I use Tomcat 8).

Singleton is never the right pattern. http://stackoverflow.com/questions/137975/what-is-so-bad-about-singletons — Samuel, Mar 07 '16 at 23:18
@Samuel Nevertheless anything bound into a namespace is a singleton by definition. — user207421, Mar 07 '16 at 23:19
@Samuel oh.. sorry I did see the enum . You dont have to synchronize here — Raghu K Nair, Mar 07 '16 at 23:26
Just wondering if there is an exception in creating ConnectionPool you may get a NPE as ds is null — Raghu K Nair, Mar 07 '16 at 23:28
See [is javax.sql.DataSource thread safe?](http://stackoverflow.com/questions/14872685/is-javax-sql-datasource-thread-safe). — , Mar 07 '16 at 23:30
@Samuel I agree with you in strict terms but that is common pattern used in lazy loading. enum are better than singleton pattern but cannot be used in all situations. Singlton with double null check and volatile instance seems to be decent implementation — Raghu K Nair, Mar 07 '16 at 23:54

AdamSkywalker · Accepted Answer · 2016-03-07T23:25:53.567

7

Though it is not explicitly mentioned in java docs, DataSource#getConnection method is designed to be called concurrently on a single DataSource instance.

You do not need to provide client-level synchronization in general case.

Restriction in java docs is probably omitted to provide flexibility for implementations.

edited Mar 07 '16 at 23:25

answered Mar 07 '16 at 23:17

AdamSkywalker

11,408
3
38
76

["that said"](https://en.wiktionary.org/wiki/that_said) generally introduces a counterpoint to your previous statement. – shmosel Mar 07 '16 at 23:21
@shmosel thank you, I thought it has a different meaning – AdamSkywalker Mar 07 '16 at 23:23

score 1 · Answer 2 · answered Mar 07 '16 at 23:22

1

Well, you shouldn't have this "ConnectionPool" in the first place. Consider using a tested one that actually does connection pooling.

Anyways, the thread-safety of your class is that of DataSource because you delegating to it and it is the only shared state you have. There is nothing in the JDBC API that ensures thread-safety and hence you need to check the implementation details.

answered Mar 07 '16 at 23:22

Sleiman Jneidi

22,907
14
56
77

I must use DOA classes so invoke lookup() in each one doesn't seem right – Vitalii Vitrenko Mar 07 '16 at 23:37
Well, you can use a library, inject a datasource instead if the singelton anti-pattern – Sleiman Jneidi Mar 07 '16 at 23:42
Thanks for tip I didn't know about recourse injection. – Vitalii Vitrenko Mar 08 '16 at 00:18

Should I synchronize getConnection() of DataSource

2 Answers2