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

[WFCORE-2080] Instantiating the WildFlyElytronProvider is an expensive operation so only do it once. #2003

Merged
merged 2 commits into from Dec 7, 2016

Conversation

darranl
Copy link
Contributor

@darranl darranl commented Dec 2, 2016

❗ Commits from this branch are used in numerous other topic branches so please do not cherry-pick or rebase. ❗

https://issues.jboss.org/browse/WFCORE-2080

@@ -97,6 +97,8 @@
*/
public class SecurityRealmService implements Service<SecurityRealm>, SecurityRealm {

private static final Provider ELYTRON_PROVIDER = new WildFlyElytronProvider();
Copy link
Contributor

Choose a reason for hiding this comment

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

This could use a comment as to why it's ok to store a mutable class in a static field. (I looked at how it's used and it's only read after construction so it's ok, but 4 years from now someone else will ask the same question.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thinking about this I can probably get away with moving this into the service lifecycle, the reported problem was it was being created per request.

Copy link
Contributor

Choose a reason for hiding this comment

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

As long as it is not created in both providers directly it is fine.
I was testing locally by just having
Provider[] providers = new Provider[]{new WildFlyElytronProvider()}
in service code and than returning that in both providers.

…e operation so only do it once during service start-up.
@wildfly-ci
Copy link

Windows Build 4331 outcome was FAILURE using a merge of 0453d06
Summary: Tests failed: 1 (1 new), passed: 3863, ignored: 74 Build time: 01:06:26

Failed tests

org.jboss.as.controller.test.TransactionalProtocolClientTestCase.testConcurrentGroup: java.net.BindException: Address already in use: bind
	at sun.nio.ch.Net.bind0(Native Method)
	at sun.nio.ch.Net.bind(Net.java:433)
	at sun.nio.ch.Net.bind(Net.java:425)
	at sun.nio.ch.ServerSocketChannelImpl.bind(ServerSocketChannelImpl.java:223)
	at sun.nio.ch.ServerSocketAdaptor.bind(ServerSocketAdaptor.java:74)
	at sun.nio.ch.ServerSocketAdaptor.bind(ServerSocketAdaptor.java:67)
	at org.xnio.nio.NioXnioWorker.createTcpConnectionServer(NioXnioWorker.java:192)
	at org.xnio.XnioWorker.createStreamConnectionServer(XnioWorker.java:243)
	at org.jboss.remoting3.remote.RemoteConnectionProvider$ProviderInterface.createServer(RemoteConnectionProvider.java:349)
	at org.jboss.as.controller.support.ChannelServer.create(ChannelServer.java:92)
	at org.jboss.as.controller.test.TransactionalProtocolClientTestCase.startChannelServer(TransactionalProtocolClientTestCase.java:108)java.lang.NullPointerException: null
	at org.jboss.as.controller.test.TransactionalProtocolClientTestCase.stopChannels(TransactionalProtocolClientTestCase.java:142)


@wildfly-ci
Copy link

Full integration - Windows Build 2396 outcome was FAILURE using a merge of 0453d06
Summary: Tests failed: 6 (6 new), passed: 3454, ignored: 116 Build time: 01:17:11

Failed tests

org.jboss.as.test.integration.domain.mixed.eap640.KernelBehavior640TestSuite: java.lang.AssertionError: expected:<EAP_6_3_0> but was:<EAP_6_4_0>
	at org.jboss.as.test.integration.domain.mixed.MixedDomainTestSuite.getVersion(MixedDomainTestSuite.java:45)
	at org.jboss.as.test.integration.domain.mixed.MixedDomainTestSuite.getSupport(MixedDomainTestSuite.java:83)
	at org.jboss.as.test.integration.domain.mixed.KernelBehaviorTestSuite.getSupport(KernelBehaviorTestSuite.java:40)
	at org.jboss.as.test.integration.domain.mixed.eap640.KernelBehavior640TestSuite.initializeDomain(KernelBehavior640TestSuite.java:42)


org.jboss.as.test.integration.domain.mixed.eap640.LegacyConfig640TestSuite: java.lang.AssertionError: expected:<EAP_6_3_0> but was:<EAP_6_4_0>
	at org.jboss.as.test.integration.domain.mixed.MixedDomainTestSuite.getVersion(MixedDomainTestSuite.java:45)
	at org.jboss.as.test.integration.domain.mixed.MixedDomainTestSuite.getSupport(MixedDomainTestSuite.java:83)
	at org.jboss.as.test.integration.domain.mixed.MixedDomainTestSuite.getSupportForLegacyConfig(MixedDomainTestSuite.java:75)
	at org.jboss.as.test.integration.domain.mixed.eap640.LegacyConfig640TestSuite.initializeDomain(LegacyConfig640TestSuite.java:37)


org.jboss.as.test.integration.domain.mixed.eap640.MixedDomain640TestSuite: java.lang.AssertionError: expected:<EAP_6_3_0> but was:<EAP_6_4_0>
	at org.jboss.as.test.integration.domain.mixed.MixedDomainTestSuite.getVersion(MixedDomainTestSuite.java:45)
	at org.jboss.as.test.integration.domain.mixed.MixedDomainTestSuite.getSupport(MixedDomainTestSuite.java:83)
	at org.jboss.as.test.integration.domain.mixed.MixedDomainTestSuite.getSupport(MixedDomainTestSuite.java:59)
	at org.jboss.as.test.integration.domain.mixed.eap640.MixedDomain640TestSuite.initializeDomain(MixedDomain640TestSuite.java:44)


org.jboss.as.test.integration.domain.mixed.eap700.KernelBehavior700TestSuite: java.lang.AssertionError: expected:<EAP_6_3_0> but was:<EAP_7_0_0>
	at org.jboss.as.test.integration.domain.mixed.MixedDomainTestSuite.getVersion(MixedDomainTestSuite.java:45)
	at org.jboss.as.test.integration.domain.mixed.MixedDomainTestSuite.getSupport(MixedDomainTestSuite.java:83)
	at org.jboss.as.test.integration.domain.mixed.KernelBehaviorTestSuite.getSupport(KernelBehaviorTestSuite.java:40)
	at org.jboss.as.test.integration.domain.mixed.eap700.KernelBehavior700TestSuite.initializeDomain(KernelBehavior700TestSuite.java:42)


org.jboss.as.test.integration.domain.mixed.eap700.LegacyConfig700TestSuite: java.lang.AssertionError: expected:<EAP_6_3_0> but was:<EAP_7_0_0>
	at org.jboss.as.test.integration.domain.mixed.MixedDomainTestSuite.getVersion(MixedDomainTestSuite.java:45)
	at org.jboss.as.test.integration.domain.mixed.MixedDomainTestSuite.getSupport(MixedDomainTestSuite.java:83)
	at org.jboss.as.test.integration.domain.mixed.MixedDomainTestSuite.getSupportForLegacyConfig(MixedDomainTestSuite.java:75)
	at org.jboss.as.test.integration.domain.mixed.eap700.LegacyConfig700TestSuite.initializeDomain(LegacyConfig700TestSuite.java:37)


org.jboss.as.test.integration.domain.mixed.eap700.MixedDomain700TestSuite: java.lang.AssertionError: expected:<EAP_6_3_0> but was:<EAP_7_0_0>
	at org.jboss.as.test.integration.domain.mixed.MixedDomainTestSuite.getVersion(MixedDomainTestSuite.java:45)
	at org.jboss.as.test.integration.domain.mixed.MixedDomainTestSuite.getSupport(MixedDomainTestSuite.java:83)
	at org.jboss.as.test.integration.domain.mixed.MixedDomainTestSuite.getSupport(MixedDomainTestSuite.java:59)
	at org.jboss.as.test.integration.domain.mixed.eap700.MixedDomain700TestSuite.initializeDomain(MixedDomain700TestSuite.java:44)


@bstansberry bstansberry added the ready-for-merge This PR is ready to be merged and fulfills all requirements label Dec 6, 2016
@bstansberry bstansberry merged commit 40c7541 into wildfly:master Dec 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to be merged and fulfills all requirements
Projects
None yet
4 participants