Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WFLY-30 SFSB containing injected DataSource fails to passivate/serialize #6925

Merged
merged 1 commit into from Nov 13, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -155,7 +155,7 @@ public void getResourceValue(final ResolutionContext context, final ServiceBuild
jndiName, false, false, Defaults.CONNECTABLE, Defaults.TRACKING, properties,
className, null, null,
xaPool, null);
final XaDataSourceService xds = new XaDataSourceService(jndiName, module.getClassLoader());
final XaDataSourceService xds = new XaDataSourceService(jndiName, jndiName, module.getClassLoader());
xds.getDataSourceConfigInjector().inject(dataSource);
startDataSource(xds, jndiName, eeModuleDescription, context, phaseContext.getServiceTarget(), serviceBuilder, injector);
} else {
Expand All @@ -165,7 +165,7 @@ public void getResourceValue(final ResolutionContext context, final ServiceBuild
Defaults.PREFILL, Defaults.USE_STRICT_MIN, Defaults.FLUSH_STRATEGY, Boolean.FALSE, null, null);
final ModifiableDataSource dataSource = new ModifiableDataSource(url, null, className, null, transactionIsolation(), properties,
null, dsSecurity, null, null, null, null, null, false, poolName, true, jndiName, Defaults.SPY, Defaults.USE_CCM, transactional, Defaults.CONNECTABLE, Defaults.TRACKING, commonPool);
final LocalDataSourceService ds = new LocalDataSourceService(jndiName, module.getClassLoader());
final LocalDataSourceService ds = new LocalDataSourceService(jndiName, jndiName, module.getClassLoader());
ds.getDataSourceConfigInjector().inject(dataSource);
startDataSource(ds, jndiName, eeModuleDescription, context, phaseContext.getServiceTarget(), serviceBuilder, injector);
}
Expand Down
Expand Up @@ -131,7 +131,7 @@ public void deploy(DeploymentPhaseContext phaseContext) throws DeploymentUnitPro
if (ds.isEnabled() && ds.getDriver() != null) {
try {
final String jndiName = Util.cleanJndiName(ds.getJndiName(), ds.isUseJavaContext());
LocalDataSourceService lds = new LocalDataSourceService(jndiName);
LocalDataSourceService lds = new LocalDataSourceService(jndiName, jndiName);
lds.getDataSourceConfigInjector().inject(buildDataSource(ds));
final String dsName = ds.getJndiName();
final PathAddress addr = getDataSourceAddress(dsName, deploymentUnit, false);
Expand All @@ -153,7 +153,7 @@ public void deploy(DeploymentPhaseContext phaseContext) throws DeploymentUnitPro
if (xads.isEnabled() && xads.getDriver() != null) {
try {
String jndiName = Util.cleanJndiName(xads.getJndiName(), xads.isUseJavaContext());
XaDataSourceService xds = new XaDataSourceService(jndiName);
XaDataSourceService xds = new XaDataSourceService(jndiName, jndiName);
xds.getDataSourceConfigInjector().inject(buildXaDataSource(xads));
final String dsName = xads.getJndiName();
final PathAddress addr = getDataSourceAddress(dsName, deploymentUnit, true);
Expand Down
Expand Up @@ -148,7 +148,7 @@ private void performRuntime(final OperationContext context, final ModelNode oper
driverDemanderBuilder.addListener(verificationHandler);
driverDemanderBuilder.setInitialMode(ServiceController.Mode.ACTIVE);

AbstractDataSourceService dataSourceService = createDataSourceService(dsName);
AbstractDataSourceService dataSourceService = createDataSourceService(dsName, jndiName);

final ManagementResourceRegistration registration = context.getResourceRegistrationForUpdate();

Expand Down Expand Up @@ -197,7 +197,7 @@ protected abstract void startConfigAndAddDependency(ServiceBuilder<?> dataSource

protected abstract void populateModel(final ModelNode operation, final ModelNode model) throws OperationFailedException;

protected abstract AbstractDataSourceService createDataSourceService(final String jndiName) throws OperationFailedException;
protected abstract AbstractDataSourceService createDataSourceService(final String dsName, final String jndiName) throws OperationFailedException;

static void populateAddModel(final ModelNode operation, final ModelNode modelNode,
final String connectionPropertiesProp, final SimpleAttributeDefinition[] attributes, PropertiesAttributeDefinition[] properties) throws OperationFailedException {
Expand Down
Expand Up @@ -111,19 +111,21 @@ public abstract class AbstractDataSourceService implements Service<DataSource> {
private final InjectedValue<ResourceAdapterRepository> raRepository = new InjectedValue<ResourceAdapterRepository>();


private final String dsName;
private final String jndiName;

protected CommonDeployment deploymentMD;
private javax.sql.DataSource sqlDataSource;
private WildFlyDataSource sqlDataSource;

/**
* The class loader to use. If null the Driver class loader will be used instead.
*/
private final ClassLoader classLoader;

protected AbstractDataSourceService(final String jndiName, final ClassLoader classLoader) {
this.jndiName = jndiName;
protected AbstractDataSourceService(final String dsName, final String jndiName, final ClassLoader classLoader ) {
this.dsName = dsName;
this.classLoader = classLoader;
this.jndiName = jndiName;
}

public synchronized void start(StartContext startContext) throws StartException {
Expand All @@ -134,10 +136,10 @@ public synchronized void start(StartContext startContext) throws StartException
if (deploymentMD.getCfs().length != 1) {
throw ConnectorLogger.ROOT_LOGGER.cannotStartDs();
}
sqlDataSource = (javax.sql.DataSource) deploymentMD.getCfs()[0];
sqlDataSource = new WildFlyDataSource((javax.sql.DataSource) deploymentMD.getCfs()[0], jndiName);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any chance use code could rely on the datasource being assignable to some other class?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should not

DS_DEPLOYER_LOGGER.debugf("Adding datasource: %s", deploymentMD.getCfJndiNames()[0]);
} catch (Throwable t) {
throw ConnectorLogger.ROOT_LOGGER.deploymentError(t, jndiName);
throw ConnectorLogger.ROOT_LOGGER.deploymentError(t, dsName);
}
}

Expand Down Expand Up @@ -342,7 +344,7 @@ public CommonDeployment deploy(ServiceContainer serviceContainer) throws DeployE
dataSources = new DatasourcesImpl(null, Arrays.asList(xaDataSourceConfig), drivers);
}

CommonDeployment c = createObjectsAndInjectValue(new URL("file://DataSourceDeployment"), jndiName,
CommonDeployment c = createObjectsAndInjectValue(new URL("file://DataSourceDeployment"), dsName,
"uniqueJdbcLocalId", "uniqueJdbcXAId", dataSources, AbstractDataSourceService.class.getClassLoader());
return c;
} catch (MalformedURLException e) {
Expand Down Expand Up @@ -608,7 +610,7 @@ private void setMcfProperties(final BaseWrapperManagedConnectionFactory managedC
}
}

// Override this method to change how jndiName is build in AS7
// Override this method to change how dsName is build in AS7
@Override
protected String buildJndiName(String rawJndiName, Boolean javaContext) {
final String jndiName;
Expand Down
Expand Up @@ -45,8 +45,8 @@ protected void populateModel(ModelNode operation, ModelNode model) throws Operat
populateAddModel(operation, model, CONNECTION_PROPERTIES.getName(), DATASOURCE_ATTRIBUTE, DATASOURCE_PROPERTIES_ATTRIBUTES);
}

protected AbstractDataSourceService createDataSourceService(final String jndiName) throws OperationFailedException {
return new LocalDataSourceService(jndiName);
protected AbstractDataSourceService createDataSourceService(final String dsName,final String jndiName) throws OperationFailedException {
return new LocalDataSourceService(dsName, jndiName);
}

@Override
Expand Down
Expand Up @@ -35,12 +35,12 @@ public class LocalDataSourceService extends AbstractDataSourceService {

private final InjectedValue<ModifiableDataSource> dataSourceConfig = new InjectedValue<ModifiableDataSource>();

public LocalDataSourceService(final String jndiName, final ClassLoader classLoader) {
super(jndiName, classLoader);
public LocalDataSourceService(final String dsName, final String jndiName, final ClassLoader classLoader) {
super(dsName, jndiName, classLoader);
}

public LocalDataSourceService(final String jndiName) {
super(jndiName, null);
public LocalDataSourceService(final String dsName, final String jndiName) {
super(dsName, jndiName, null);
}

@Override
Expand Down
@@ -0,0 +1,146 @@
/*
* JBoss, Home of Professional Open Source.
* Copyright 2014, Red Hat, Inc., and individual contributors
* as indicated by the @author tags. See the copyright.txt file in the
* distribution for a full listing of individual contributors.
*
* This is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as
* published by the Free Software Foundation; either version 2.1 of
* the License, or (at your option) any later version.
*
* This software is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this software; if not, write to the Free
* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
*/

package org.jboss.as.connector.subsystems.datasources;

import java.io.IOException;
import java.io.PrintWriter;
import java.io.Serializable;
import java.sql.Connection;
import java.sql.SQLException;
import java.sql.SQLFeatureNotSupportedException;
import java.util.logging.Logger;

import javax.naming.InitialContext;
import javax.sql.DataSource;

/**
* WildFly DataSource implementation
*
* @author <a href="jesper.pedersen@redhat.com">Jesper Pedersen</a>
*/
public class WildFlyDataSource implements DataSource, Serializable {

/** The serialVersionUID */
private static final long serialVersionUID = 1L;

/** DataSource */
private transient DataSource delegate;

/** Service name */
private transient String jndiName;


/**
* Constructor
* @param delegate The datasource
* @param jndiName The service name
*/
public WildFlyDataSource(DataSource delegate, String jndiName) {
this.delegate = delegate;
this.jndiName = jndiName;
}

/**
* {@inheritDoc}
*/
public Connection getConnection() throws SQLException {
return delegate.getConnection();
}

/**
* {@inheritDoc}
*/
public Connection getConnection(String username, String password) throws SQLException {
return delegate.getConnection(username, password);
}

/**
* {@inheritDoc}
*/
public <T> T unwrap(Class<T> iface) throws SQLException {
throw new SQLException("Unwrap not supported");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not delegate this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Becose delegate isn't wrapper for interface, as next method is saying

}

/**
* {@inheritDoc}
*/
public boolean isWrapperFor(Class<?> iface) throws SQLException {
return false;
}

/**
* {@inheritDoc}
*/
public PrintWriter getLogWriter() throws SQLException {
return delegate.getLogWriter();
}

/**
* {@inheritDoc}
*/
public void setLogWriter(PrintWriter out) throws SQLException {
delegate.setLogWriter(out);
}

/**
* {@inheritDoc}
*/
public void setLoginTimeout(int seconds) throws SQLException {
delegate.setLoginTimeout(seconds);
}

/**
* {@inheritDoc}
*/
public int getLoginTimeout() throws SQLException {
return delegate.getLoginTimeout();
}

/**
* {@inheritDoc}
*/
public Logger getParentLogger() throws SQLFeatureNotSupportedException {
return delegate.getParentLogger();
}

private void writeObject(java.io.ObjectOutputStream out) throws IOException {
out.defaultWriteObject();
out.writeObject(jndiName);
}


private void readObject(java.io.ObjectInputStream in) throws IOException, ClassNotFoundException {
in.defaultReadObject();
jndiName = (String) in.readObject();


try {
InitialContext context = new InitialContext();

DataSource originalDs = (DataSource) context.lookup(jndiName);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't really work if the DS jndi name is not global and the entry is deserialised outside the scope of a deployment.

Probably not going to be a big issue in practice, but it could be an issue with datasources that are embedded in a deployment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but at least this solution fix almost cases. W/o this patch it's not working at all in a cluster. This is a corner case, I would suggest to just ignore this, at least if you haven't any hint for a general solution covering also corner case.

this.delegate = originalDs;
} catch (Exception e) {
throw new IOException(e);
}
}
}
Expand Up @@ -46,8 +46,8 @@ protected void populateModel(ModelNode operation, ModelNode model) throws Operat
populateAddModel(operation, model, XADATASOURCE_PROPERTIES.getName(), XA_DATASOURCE_ATTRIBUTE, XA_DATASOURCE_PROPERTIES_ATTRIBUTES);
}

protected AbstractDataSourceService createDataSourceService(final String jndiName) throws OperationFailedException {
return new XaDataSourceService(jndiName);
protected AbstractDataSourceService createDataSourceService(final String dsName, final String jndiName) throws OperationFailedException {
return new XaDataSourceService(dsName, jndiName);
}

@Override
Expand Down
Expand Up @@ -38,12 +38,12 @@ public class XaDataSourceService extends AbstractDataSourceService {

private final InjectedValue<ModifiableXaDataSource> dataSourceConfig = new InjectedValue<ModifiableXaDataSource>();

public XaDataSourceService(final String jndiName, final ClassLoader classLoader) {
super(jndiName, classLoader);
public XaDataSourceService(final String dsName, final String jndiName, final ClassLoader classLoader) {
super(dsName, jndiName, classLoader);
}

public XaDataSourceService(final String jndiName) {
this(jndiName, null);
public XaDataSourceService(final String dsName, final String jndiName) {
this(dsName, jndiName, null);
}

@Override
Expand Down
Expand Up @@ -105,7 +105,7 @@ private void testAddDataSource() throws Exception {

// check that it is available through JNDI
String jndiClass = JndiServlet.lookup(url.toString(), "java:jboss/datasources/TestDS");
Assert.assertEquals("org.jboss.jca.adapters.jdbc.WrapperDataSource", jndiClass);
Assert.assertTrue(javax.sql.DataSource.class.isAssignableFrom(Class.forName(jndiClass)));

}

Expand Down Expand Up @@ -170,7 +170,7 @@ private void testAddXaDataSource() throws Exception {

// check that it is available through JNDI
String jndiClass = JndiServlet.lookup(url.toString(), "java:jboss/datasources/TestXADS");
Assert.assertEquals("org.jboss.jca.adapters.jdbc.WrapperDataSource", jndiClass);
Assert.assertTrue(javax.sql.DataSource.class.isAssignableFrom(Class.forName(jndiClass)));


}
Expand Down
Expand Up @@ -154,9 +154,9 @@ public void simpleFailover(

/**
* Validates failover on redeploy of a @Stateful bean containing injected JDBC resource manager connection factories
* test for WFLY-30 @Resource injection of Datasource on clustered SFSB fails with serialization error
*/
@Test
@Ignore("WFLY-30 @Resource injection of Datasource on clustered SFSB fails with serialization error")
public void connectionFactoryFailover(
@ArquillianResource() @OperateOnDeployment(DEPLOYMENT_1) URL baseURL1,
@ArquillianResource() @OperateOnDeployment(DEPLOYMENT_2) URL baseURL2) throws Exception {
Expand Down