Substitute ReadWriteLock with AtomicReference for non-blocking operations

Question

I wrote this class to reload a DataSource, used by the entire application, when the persisted configuration data changes.
As you can see it is managed by CDI and exposed as a Singleton, and the "configuration changed" event arrives through the configurationReload(...) method, but that's not relevant now.

The reference update is guarded by a ReentrantReadWriteLock, but I'm wondering if it is needed at all.

@Singleton
@ThreadSafe
class ReloadingDataSource implements DataSource {
    private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock();
    private final Lock readLock = readWriteLock.readLock();
    private final Lock writeLock = readWriteLock.writeLock();

    @GuardedBy("readWriteLock")
    private DataSource delegateDataSource;

    @Inject
    ReloadingDataSource(@Nonnull final Configuration configuration) {
        delegateDataSource = createDataSource(configuration);
    }

    private DataSource createDataSource(final Configuration configuration) {
        ... Create a ComboPooledDataSource using properties extracted from Configuration.
    }

    @Override
    public Connection getConnection() throws SQLException {
        readLock.lock();

        try {
            return delegateDataSource.getConnection();
        } finally {
            readLock.unlock();
        }
    }

    ...

    private void configurationReload(
            @Observes @Reload final ConfigurationChanged configurationChanged,
            @Nonnull final Configuration configuration) {
        final ConfigurationEvent event = configurationChanged.getConfigurationEvent();

        if (event.getType() != AbstractFileConfiguration.EVENT_RELOAD && !event.isBeforeUpdate()) {
            return;
        }

        writeLock.lock();

        try {
            destroyDelegateDataSource();
            delegateDataSource = createDataSource(configuration);
        } finally {
            writeLock.unlock();
        }
    }

    private void destroyDelegateDataSource() {
        try {
            DataSources.destroy(delegateDataSource);
        } catch (final SQLException ignored) {
            // Do nothing.
        }
    }
}

If we ignore the cost of creating a new DataSource, could the above strategy be substituted by an AtomicReference<DataSource>, as below?
It would result in better performing and easier to read code.

Are there better ways to handle this which I'm not aware of?

@Singleton
@ThreadSafe
class ReloadingDataSource implements DataSource {
    private final AtomicReference<DataSource> delegateDataSource;

    @Inject
    ReloadingDataSource(@Nonnull final Configuration configuration) {
        delegateDataSource = new AtomicReference<>(createDataSource(configuration));
    }

    private DataSource createDataSource(final Configuration configuration) {
        ... Create a ComboPooledDataSource using properties extracted from Configuration.
    }

    @Override
    public Connection getConnection() throws SQLException {
        return delegateDataSource.get().getConnection();
    }

    ...

    private void configurationReload(
            @Observes @Reload final ConfigurationChanged configurationChanged,
            @Nonnull final Configuration configuration) {
        final ConfigurationEvent event = configurationChanged.getConfigurationEvent();

        if (event.getType() != AbstractFileConfiguration.EVENT_RELOAD && !event.isBeforeUpdate()) {
            return;
        }

        // Updated as per eckes tip. Is this what you meant?
        final DataSource newDataSource = createDataSource(configuration);

        while (true) {
            final DataSource oldDataSource = delegateDataSource.get();

            if (delegateDataSource.compareAndSet(oldDataSource, newDataSource)) {
                destroyDelegateDataSource(oldDataSource);
                break;
            }
        }
    }

    private void destroyDelegateDataSource(final DataSource oldDataSource) {
        try {
            DataSources.destroy(oldDataSource);
        } catch (final SQLException ignored) {
            // Do nothing.
        }
    }
}

Answer 1

If you require your updates to be handled in an ordered fashion, you still need locking on the reload method. In which case, you can ditch the AtomicReference logic and just go with a volatile:

public class RDS {
  private volatile DataSource delegate;

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

  private void reload(Configuration config) {
    DataSource old = null;
    synchronized(this) {
      old = delegate;
      delegate = createDataSource(config);
    }
    destroyDataSource(old);
  }
}

Note however, you still potentially have other issues where connections could still be in use for the old DataSource when you close it (mentioned in @eckes first comment on the question). In order to fix that, you need something like a connection Pool with acquire/release type logic which closes the old delegate once all existing connections have been released.