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-3765] Remove org.jboss.msc from the modular system class path #3244

Merged
merged 1 commit into from
May 4, 2018

Conversation

jamezp
Copy link
Member

@jamezp jamezp commented May 2, 2018

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

Follows up on #3229 to declare remove org.jboss.msc from the modular system class path.

@jamezp jamezp requested a review from bstansberry May 2, 2018 22:42
Copy link
Member

@dmlloyd dmlloyd left a comment

Choose a reason for hiding this comment

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

So the change is just to make it not be provided?

@jamezp
Copy link
Member Author

jamezp commented May 2, 2018

@dmlloyd Correct. It's required on the class path for the ManagedProcessFactory.createXxx() to work.

@bstansberry
Copy link
Contributor

bstansberry commented May 3, 2018

@jamezp Can you link where this is used on the embedding app side? I'm not seeing it.

edit: I do see this, where org.jboss.msc is being included as a system package on the JBM side:

src/main/java/org/wildfly/core/embedded/EmbeddedProcessFactory.java:            StringBuilder packages = new StringBuilder("org.jboss.modules,org.jboss.msc,org.jboss.dmr,org.jboss.threads,org.jboss.as.controller.client");

So that would probably necessitate this change. I don't see why that package needs to be in the list though; nothing from it is used internally on the embedding app side, and nor is anything from it exposed to caller. It might be cruft. I don't know if removing that would somehow be a breaking change though. I can't think how it would be.

@jamezp
Copy link
Member Author

jamezp commented May 3, 2018

@bstansberry Yes that's where it's at. It's trying to load MSC from the modular system class loader.

Exception in thread "main" java.lang.IllegalStateException: WFLYEMB0019: Cannot create standalone server using factory: public static org.wildfly.core.embedded.StandaloneServer org.wildfly.core.embedded.EmbeddedStandaloneServerFactory.create(java.io.File,org.jboss.modules.ModuleLoader,java.util.Properties,java.util.Map,java.lang.String[])
	at org.wildfly.core.embedded.EmbeddedProcessFactory.createManagedProcess(EmbeddedProcessFactory.java:301)
	at org.wildfly.core.embedded.EmbeddedProcessFactory.createStandaloneServer(EmbeddedProcessFactory.java:183)
	at org.wildfly.reproducer.Main.main(Main.java:57)
Caused by: java.lang.NoClassDefFoundError: org/jboss/msc/value/Value
	at org.wildfly.core.embedded.EmbeddedStandaloneServerFactory.create(EmbeddedStandaloneServerFactory.java:101)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.wildfly.core.embedded.EmbeddedProcessFactory.createManagedProcess(EmbeddedProcessFactory.java:296)
	... 2 more
Caused by: java.lang.ClassNotFoundException: org.jboss.msc.value.Value
	at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
	at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:349)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	at org.jboss.modules.JDKSpecific.getSystemClass(JDKSpecific.java:180)
	at org.jboss.modules.ConcurrentClassLoader.performLoadClass(ConcurrentClassLoader.java:395)
	at org.jboss.modules.ConcurrentClassLoader.loadClass(ConcurrentClassLoader.java:116)
	... 8 more

@bstansberry
Copy link
Contributor

@jamezp It's cruft left over from the pre-WildFly Core embedded, where StandaloneServer exposed MSC:

https://github.com/jbossas/jboss-eap/blob/6.x/embedded/src/main/java/org/jboss/as/embedded/StandaloneServer.java#L64

That got removed along the way.

Unless @dmlloyd can think of a way that the MSC lib used by the appserver also being visible to the embedding app would be useable (e.g. some static methods in MSC that would let the embedding app hack into the appserver ServiceContainer), I think we should just remove org.jboss.msc from the system package list. And even if there is a way to hack like that we probably should cut it off.

@jamezp
Copy link
Member Author

jamezp commented May 3, 2018

@bstansberry I think that makes sense. I'll place a hold label on this until we make a decision the.

@jamezp jamezp added the hold Do not merge this PR label May 3, 2018
@dmlloyd
Copy link
Member

dmlloyd commented May 4, 2018

I'm OK with removing it.

…ed for the library to be on the API's class path.
@jamezp jamezp removed the hold Do not merge this PR label May 4, 2018
@jamezp jamezp changed the title [WFCORE-3765] Add MSC to the embedded apps class path. [WFCORE-3765] Remove org.jboss.msc from the modular system class path May 4, 2018
@jamezp
Copy link
Member Author

jamezp commented May 4, 2018

The dependency change has been removed and org.jboss.msc package has been removed from the modular system packages.

@wildfly-ci
Copy link

Core - Full Integration Build 7287 outcome was FAILURE using a merge of 7816507
Summary: Execution timeout (new); tests passed: 4174, ignored: 136 Build time: 02:30:48

@jamezp jamezp merged commit a05cd8d into wildfly:master May 4, 2018
@jamezp jamezp deleted the WFCORE-3765 branch March 13, 2019 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants